Skip to content

Add emacs config example - #1263

Open
bigbuger wants to merge 1 commit into
EmmyLuaLs:mainfrom
bigbuger:patch-1
Open

bigbuger wants to merge 1 commit into
EmmyLuaLs:mainfrom
bigbuger:patch-1

Conversation

@bigbuger

@bigbuger bigbuger commented Sep 24, 2026 •

Copy link
Copy Markdown

Add emacs config example, via lsp-mode

@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: README.md Changes

Summary

This PR adds an Emacs lsp-mode configuration example for the emmylua_ls language server. The documentation is generally well-structured and follows the existing <details> pattern used elsewhere in the README.

Issues Found

1. Trailing Whitespace / Tab Character

Line after <summary><b>Emacs</b></summary>:

lsp-mode example:
	

There is a line containing only a tab character (\t) between the summary and the code block. This is a whitespace-only line that will likely trigger linter warnings and is inconsistent with the rest of the file.

Recommendation: Remove the tab and use a single blank line (or no blank line) for consistency.

2. Extra Blank Line Before Next <details>

+</details>
+
+
 <details>
 <summary><b>Other editors</b></summary>

Two blank lines were added before the "Other editors" section, while other sections in the file appear to use a single blank line separator.

Recommendation: Reduce to a single blank line to match the surrounding style.

3. Missing Language Identifier on Code Fence (Minor)

The code block uses ```elisp, which is correct. However, verify consistency with other editor examples in the README — if they use ```lua, ```json, etc., this is fine. No action needed if consistent.

4. Potential Functional Concern — cargo install Assumption

"cargo" "install" "emmylua_ls"

The download function assumes emmylua_ls is installable via cargo install emmylua_ls. Please verify:

  • The actual crate name on crates.io (it may differ from the binary name, e.g., emmylua_ls vs emmylua-ls).
  • Whether cargo is guaranteed to be on the user's PATH.

If the crate name is incorrect, users following this doc will get a build failure. Consider adding a brief note or link to installation instructions.

5. :priority 0 May Be Intentional — Worth a Comment

:priority 0

A priority of 0 is lower than the default lsp-mode clients. If this is intentional (to avoid overriding other Lua servers), it's fine, but a short comment explaining the choice would help users understand.

6. lsp-activate-on "lua" vs lua-mode

The activation function uses "lua" (language ID), while the hook uses lua-mode. These are consistent with lsp-mode conventions, so no change needed — just flagging for reviewer awareness.

Recommendations

  • Required: Remove the tab-only line and the extra blank line.
  • Suggested: Verify the cargo install crate name and add a note if installation differs from the binary name.
  • Optional: Add a brief comment about :priority 0 if it's a deliberate choice.

Verdict

Minor formatting cleanup needed before merge; the functional content looks reasonable assuming the cargo install target is correct.

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.

1 participant