Skip to content

Add more information about formatter to docs#1124

Merged
CppCXY merged 2 commits into
EmmyLuaLs:mainfrom
colin-heffernan:main
Jun 23, 2026
Merged

Add more information about formatter to docs#1124
CppCXY merged 2 commits into
EmmyLuaLs:mainfrom
colin-heffernan:main

Conversation

@colin-heffernan

Copy link
Copy Markdown
Contributor

For being a seemingly-major part of the toolchain, emmylua_formatter/luafmt wasn't explicitly named in README.md or CONTRIBUTING.md.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

This PR adds documentation for a new emmylua_formatter crate. The changes are clean and well-structured. Here are my observations:

✅ Positive Aspects

  • Consistent formatting with existing documentation style
  • Proper table alignment in CONTRIBUTING.md
  • Clear, concise descriptions
  • Good placement of new content in logical order

🔍 Issues Found

  1. Missing crate description in README.md

    • The emmylua_formatter crate is listed in the install section but not in the crate overview table at the top of README.md
    • Suggestion: Add a row for emmylua_formatter in the crate table for consistency
  2. Inconsistent command example

    • The formatter example uses luafmt while the crate is named emmylua_formatter
    • Suggestion: Either clarify that luafmt is an alias, or use emmylua_formatter in the example for consistency
  3. Missing formatter options documentation

    • Other tools show available flags/options, but the formatter only shows one example
    • Suggestion: Add a table of available options (e.g., --write, --check, config file support)

📝 Minor Suggestions

  • Consider adding a link to the formatter's own documentation or configuration guide
  • The formatter description in CONTRIBUTING.md could be more descriptive about its features (e.g., "Configurable Lua code formatter with customizable style rules")

🏁 Overall Assessment

Approved with minor suggestions. The changes are functionally correct and maintain the project's documentation standards. The issues noted are non-blocking improvements.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the documentation in README.md and CONTRIBUTING.md to introduce the new emmylua_formatter crate, including its installation via Cargo and usage instructions using the luafmt command. The review feedback suggests improving visual clarity in the crates table by using a distinct emoji for the formatter, and clarifying that installing emmylua_formatter provides the luafmt binary.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread CONTRIBUTING.md Outdated
Comment thread README.md
```bash
# Via Cargo
cargo install emmylua_ls # Language server
cargo install emmylua_formatter # Code formatter

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since installing the emmylua_formatter crate actually provides the luafmt binary, it would be helpful to explicitly mention luafmt in the comment to avoid confusion for users looking for an emmylua_formatter executable.

Suggested change
cargo install emmylua_formatter # Code formatter
cargo install emmylua_formatter # Code formatter (luafmt)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this, since nothing else follows this pattern, but nothing else needs to. Will wait for human feedback on this one.

@CppCXY CppCXY merged commit b80d09a into EmmyLuaLs:main Jun 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants