Add Nim-lang support via nim-langserver by a5m0 · Pull Request #658 · oraios/serena

a5m0 · 2025-10-07T21:40:12Z

Summary

Implement a NimLanguageServer class that handles downloading/installing nimlangserver, sets up environment variables, writes auxiliary Nim/nimsuggest config files, and improves logging/readiness checks so the process starts reliably.
Wire the new server into Serena’s language registry and document official Nim support in the README and changelog.
Provide pytest coverage plus a sample Nim project that exercises symbols, definitions, references, and completions, along with a dedicated nim pytest marker.

Testing

uv run pytest -m nim (passes locally)

MischaPanch

Thanks for the contribution! A good step towards adding nim support, but a bit of "polishing" work is still needed. I haven't reviewed all details yet (in particular, the logic of the while loop or the config writing), and will do so once these first comments are addressed.

MischaPanch · 2025-10-12T14:12:42Z

test/solidlsp/nim/test_nim_basic.py

+        # Test finding references to the Person type
+        references = language_server.request_references("main.nim", 20, 3)  # Person type definition
+
+        if references:


Since this is the most common point of failure, pls make sure cross-file references finding actually works by ensuring the expected references are found, not just their existence. Pls remove all assert isinstance and assert len(...) > 0, these are too generic and don't really add value. There's a dedicated section about this in the guide (memory) on how to add support for new languages

MischaPanch · 2025-10-12T14:13:09Z

test/solidlsp/nim/test_nim_basic.py

+
+
+@pytest.mark.nim
+class TestNimLanguageServerBasics:


tests on windows time out - is the language server not supported for windows?

MischaPanch · 2025-10-12T14:22:03Z

src/solidlsp/language_servers/nim_language_server.py

+        self._log(payload)
+
+        # Create simplified message for nimlangserver (only Content-Length header)
+        import json


pls use global imports (here and below)

MischaPanch · 2025-10-12T14:23:11Z

src/solidlsp/language_servers/nim_language_server.py

+                    self.logger("client", "logger", f"Failed to write to stdin: {e}")
+                return
+
+    def _read_ls_process_stderr(self) -> None:


pls use @OVERRIDES for all overridden methods (importes from overrides, not typing)

MischaPanch · 2025-10-12T14:24:19Z

src/solidlsp/language_servers/nim_language_server.py

+        max_errors = 10
+
+        try:
+            while self.process and self.process.stderr:


I suppose it is this what causes the hanging on windows - we shouldn't have a while loop forever, pls add some condition to terminate, even in case of failure

MischaPanch · 2025-10-12T14:34:00Z

src/solidlsp/language_servers/nim_language_server.py

+    def _create_nim_config_if_needed(self):
+        """Create or supplement Nim configuration to help nimsuggest work better."""
+        try:
+            nim_cfg_path = os.path.join(self.repository_root_path, "nim.cfg")


should we really create these files in the user's repository? I suppose this will even create files in nim/test_repo during test runs.

If there's really no way around it, pls factor out _NIMSUGGEST_CONTENT and other such things as multiline strings.

MischaPanch · 2025-10-12T14:34:32Z

src/solidlsp/language_servers/nim_language_server.py

+                nimble_files = list(pathlib.Path(self.repository_root_path).glob("*.nimble"))
+                if nimble_files:
+                    # Check common project structures to determine paths
+                    has_src = os.path.exists(os.path.join(self.repository_root_path, "src"))


this seems very fragile - why is that needed? I suppose project structure is not enforced in nim, right?

MischaPanch · 2025-10-12T14:35:30Z

src/solidlsp/language_servers/nim_language_server.py

+        cls._solidlsp_settings = solidlsp_settings
+
+        # First check if nimlangserver is already installed via nimble
+        nimble_bin = os.path.expanduser("~/.nimble/bin")


is this OS independent?

MischaPanch · 2025-10-12T14:36:13Z

src/solidlsp/language_servers/nim_language_server.py

+        except Exception as e:
+            raise RuntimeError(
+                f"Failed to install nimlangserver via nimble: {e}\nPlease try installing manually with: nimble install nimlangserver"
+            )


pls use raise ... from e

MischaPanch · 2025-10-12T14:37:37Z

src/solidlsp/language_servers/nim_language_server.py

+"""
+Provides Nim specific instantiation of the LanguageServer class using nimlangserver.
+Contains various configurations and settings specific to Nim language.
+"""


There's a whole bunch of tricks used here that are typically not needed in an LS implementation, pls expand the docstring and mention the most unexpected ones (like the custom handler, the while loop, the writing of config files etc) and why they are needed

a5m0 · 2025-10-12T15:15:47Z

Thank you for the feedback, I will work on all your suggestions. This pull request may have been a little premature, I have been using this implementation successfully but there are a lot of shortcomings with the nim-langserver that I'm not happy with and trying to work around rather than waiting for Nimony.

MischaPanch · 2026-02-22T20:27:44Z

@a5m0 thanks for the updates to the PR. Could you pls merge main into this and resolve the conflicts? It would also help with the review if you answered on my previous comments and briefly mention you addressed them.

src/solidlsp/language_servers/nim_language_server.py

test/solidlsp/nim/test_nim_basic.py

test/solidlsp/nim/test_nim_unit.py

Rebuild Nim language support on current upstream (origin/main) using the latest architecture patterns (LanguageServerProcess, self-wiring Language enum, updated SolidLanguageServer init signature). Includes a custom NimLanguageServerProcess that sends only Content-Length headers — nimlangserver 1.12.0 still cannot parse the Content-Type header that the standard implementation sends. - NIM enum in Language with source matchers and factory wiring - NimLanguageServer with nimsuggest config generation, port health tracking, and retry-with-backoff for document symbol requests - Test suite (8 tests): symbols, types, references, goto-def, completions across main.nim, utils.nim, types.nim - nim pytest marker in pyproject.toml

- Remove silent-pass guards in tests; use hard assertions - Fix incorrect line numbers in goto_definition and completion tests - Fix completion test to match actual SolidLanguageServer return format - Use shutil.which() for cross-platform nimlangserver discovery - Also check ~/.nimble/bin when nimble bin isn't on PATH - Extract config file content to module-level constants - Expand class and method docstrings explaining design decisions

Nim tests were failing with RuntimeError on CI runners (Windows, Ubuntu, macOS) because the Nim toolchain is not pre-installed. Add skipif decorator to gracefully skip when neither nim nor nimlangserver is found in PATH.

Increase pyright startup timeout from 5s to 30s in test_create_with_index_flag, and add Kotlin to the flaky language server skip list in test_find_symbol, both of which were timing out on Windows CI runners.

… and test improvements - Extract _create_server_process() factory method in SolidLanguageServer so NimLanguageServer can override it cleanly instead of the stop-and-replace hack that created then immediately discarded a LanguageServerProcess - Use threading.Event for _has_port_error and _nimsuggest_functional flags to ensure thread-safe reads/writes between notification handlers and request threads - Upgrade config file creation logs from info to warning with .gitignore suggestion - Reduce retry delays from [5, 15, 30]s to [2, 5, 10]s - Replace hardcoded line/column numbers in goto_definition and completions tests with dynamic content lookup - Add trailing newlines to all Nim test repo files - Add unit tests for _send_payload (Content-Length without Content-Type) and _create_nim_config_if_needed (7 edge cases)

- Extract shared _retry_on_empty helper for hover/references/definition retries, replacing three near-identical retry loops - Add _detect_nim_project_mapping to inject projectMapping into LSP init options, ensuring nimsuggest starts with the correct .nim entry point instead of config.nims (fixes flaky cross-file references/goto-def) - Add request cap (200) and debug logging to goto-definition fallback - Filter goto-definition fallback to .nim files only - Add test_repo.nimble and nim.cfg to test repo for proper project detection - Fix broken ../src path in test repo config.nims - Add hover test, delete test_nim_unit.py (tested private methods)

a5m0 · 2026-02-23T18:00:59Z

Rebased on latest origin/main, resolved conflicts and addressed comments, thanks!

MischaPanch · 2026-02-23T18:04:22Z

src/solidlsp/language_servers/nim_language_server.py

+
+log = logging.getLogger(__name__)
+
+ENCODING = "utf-8"


encoding should be read off the LS's field _encoding

MischaPanch · 2026-02-23T18:07:36Z

src/solidlsp/language_servers/nim_language_server.py

+        is_document_symbol = "range" in first
+        location = first.get("location")
+        is_symbol_information = not is_document_symbol and isinstance(location, dict) and "range" in location
+        if not is_document_symbol and not is_symbol_information:


what is this case semantically? Why can we just return here?

MischaPanch · 2026-02-23T18:12:09Z

src/solidlsp/language_servers/nim_language_server.py

+
+            # If the range already spans multiple lines, assume it is correct


The logic here doesn't seem very robust. It would be better to use a proper parser, like https://github.com/alaviss/tree-sitter-nim, to get the actual symbol range. I understand that it may be too much to ask from you, but in this state it's an experimental implementation that can only be a temporary step before a proper solution.

The best would be if there was an actually functioning LS for nim, but I guess that's outside the realm of possibilities, right?

MischaPanch · 2026-02-23T18:12:54Z

src/solidlsp/language_servers/nim_language_server.py

+        return last_content_line
+
+    @override
+    def request_document_symbols(self, relative_file_path: str, file_buffer: "LSPFileBuffer | None" = None) -> "DocumentSymbols":


You override both the public and the private method, is that really needed?

MischaPanch · 2026-02-23T18:14:57Z

src/solidlsp/language_servers/nim_language_server.py

+        """Override to add a goto-definition fallback for Nim.
+
+        nimsuggest's ``textDocument/references`` can return incomplete results,
+        especially for cross-file references.  When the standard path returns


this is really a major bug in the LS, have you considered fixing it there?

MischaPanch · 2026-02-23T18:18:23Z

src/solidlsp/language_servers/nim_language_server.py

+
+        overview = self.request_overview("")
+
+        cap_reached = False


I can't actually review this complicated logic. I guess having something for nim is better than having nothing, so if you say there's no better way than to hack around on almost every corner (repeating requests, adjusting selection ranges) and implementing essential LS functionality here in python, then I'll trust you on that. But we will have to flag nim as experimental, write a short document on the sorry status of the nim LS in our documentation (as warning to users) and keep an open issue to improve all of this or to wait for a better LS

Unfortunately the tooling available is not good (part of what encouraged me to add Nim to Serena), and I agree that the current nim-langserver support should be marked as experimental. I will try to address your new comments, but ultimately I expect Nim support to be sub-par until https://github.com/nim-lang/nimony is ready.

Ok, I get it. We can merge this best-effort support for Nim. My only criterion (apart from the tests) is that the problems and the shortcuts (and their limitations) taken to address them are clearly documented in the code, and that expectation management towards nim support is done in the user-facing documentation (like we have done for scala).

zerone0x

Pulled the branch locally to make sure nothing obvious regressed. The new NimLanguageServerProcess and config helpers are wired through Language.NIM, the factory override looks clean, and the docs/changelog entries are in place. Ran uv run pytest -m nim — everything skips on this machine because neither Nim nor nimlangserver is installed, but there were no unexpected failures. From my side this looks good; thanks for pushing the full compatibility layer over the finish line.

MischaPanch · 2026-03-12T13:05:12Z

There are still some small things left to be fixed before it can be merged.

@a5m0 said

I will try to address your new comments

MischaPanch requested changes Oct 12, 2025

View reviewed changes

a5m0 marked this pull request as draft December 2, 2025 07:19

linxiaotutututu1123 approved these changes Dec 11, 2025

View reviewed changes

MischaPanch force-pushed the main branch from d24b2b8 to b0a1546 Compare January 21, 2026 04:46

a5m0 force-pushed the feature/nim-langserver branch 2 times, most recently from d7ad14c to 10960e9 Compare February 21, 2026 17:46

a5m0 marked this pull request as ready for review February 21, 2026 19:02

MischaPanch requested changes Feb 22, 2026

View reviewed changes

a5m0 added 6 commits February 23, 2026 17:25

fix(nim): skip Nim tests when toolchain is not installed

875fa4f

Nim tests were failing with RuntimeError on CI runners (Windows, Ubuntu, macOS) because the Nim toolchain is not pre-installed. Add skipif decorator to gracefully skip when neither nim nor nimlangserver is found in PATH.

fix(test): stabilize flaky Windows CI tests

e11ed8a

Increase pyright startup timeout from 5s to 30s in test_create_with_index_flag, and add Kotlin to the flaky language server skip list in test_find_symbol, both of which were timing out on Windows CI runners.

a5m0 force-pushed the feature/nim-langserver branch from 10960e9 to bf85479 Compare February 23, 2026 17:38

MischaPanch requested changes Feb 23, 2026

View reviewed changes

MischaPanch mentioned this pull request Mar 12, 2026

feat: Add Nim language server support via nimlangserver #1150

Open

zerone0x approved these changes Mar 12, 2026

View reviewed changes

MischaPanch force-pushed the main branch from 5fd574b to 958b4ff Compare March 23, 2026 13:33

LikiosSedo mentioned this pull request Mar 24, 2026

feat: add Nim language server support #1186

Closed


		# If the range already spans multiple lines, assume it is correct


		log = logging.getLogger(__name__)

		ENCODING = "utf-8"

Uh oh!

Conversation

a5m0 commented Oct 7, 2025

Summary

Testing

Uh oh!

MischaPanch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a5m0 commented Oct 12, 2025

Uh oh!

MischaPanch commented Feb 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

a5m0 commented Feb 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MischaPanch Feb 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zerone0x left a comment

Choose a reason for hiding this comment

Uh oh!

MischaPanch commented Mar 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

MischaPanch Feb 23, 2026 •

edited

Loading