Dead code removal and minor fixes#90
Merged
Merged
Conversation
- ocaml-eglot-req--PlainUri (unused) - ocaml-eglot-req--definition and --declaration (the xref backend uses eglot--lsp-xrefs-for-method or Merlin tunneling directly) - ocaml-eglot-util--ensure-is-interface (only --ensure-interface without the -is- variant is used) - ocaml-eglot-util--merlin-location-to-lsp (unused) - unreachable fallback 25 in ocaml-eglot--search limit or-chain (ocaml-eglot-type-search-limit is always truthy)
The else branch of the if used (when a 1) (when b -1) 0 which is an implicit progn -- only the final 0 was ever returned. When one position was nil the function always returned 0 (equal) instead of indicating ordering. Use cond to handle each case properly.
find-file switches the displayed buffer (causing visible flicker) and triggers find-file-hook. Since the intent is just ensuring the file is loaded into a buffer, find-file-noselect is sufficient and avoids both issues.
The function checked the extension on buffer-file-name, then got the URI, converted it back to a path, and checked the same extension again via is-interface. The inner call was always redundant. Also avoids the unnecessary eglot call when the buffer has no file or isn't an interface.
The type display buffer called (funcall current-major-mode) on every update (grow, shrink, verbosity change), which re-runs all mode hooks. Only switch when the mode has actually changed.
Member
|
Thanks a lot! |
Contributor
Author
|
Sure! I don't normally add changelog entries in my projects for changes that are internal (not user-visible), but I can certainly add something here (and for #91 as well). |
bbatsov
added a commit
to bbatsov/ocaml-eglot
that referenced
this pull request
Mar 2, 2026
bbatsov
added a commit
to bbatsov/ocaml-eglot
that referenced
this pull request
Mar 2, 2026
bbatsov
added a commit
to bbatsov/ocaml-eglot
that referenced
this pull request
Mar 2, 2026
bbatsov
force-pushed
the
cleanup-dead-code-and-minor-fixes
branch
from
ece9d8c to
1f18f70
Compare
bbatsov
added a commit
to bbatsov/ocaml-eglot
that referenced
this pull request
Mar 2, 2026
Contributor
Author
|
I've added changelog entries under a new "Unreleased" section as requested. These will likely cause a minor merge conflict with #91 once one of them lands, but that's easy to resolve. |
xvw
approved these changes
Mar 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I noticed a few unused functions while going through the codebase -- I'm not sure what their original purpose was, but they seem to have become obsolete at some point (likely after the xref backend rewrite), so I cleaned them up. The removed functions are:
ocaml-eglot-req--PlainUriocaml-eglot-req--definitionand--declarationocaml-eglot-util--ensure-is-interfaceocaml-eglot-util--merlin-location-to-lsp25fallback in the search limitorchainOn top of that there are a few small fixes:
compare-positionhad a bug in the nil handling branch (thewhenreturn values were silently discarded in an implicit progn, so the function always returned 0 when either position was nil)load-uriusedfind-file+switch-to-bufferinstead offind-file-noselect, causing visible buffer flicker and spurious hook triggerson-interfacechecked the file extension twice (once onbuffer-file-name, then again via URI conversion)progninfind-identifier-in-alternate-file