Skip to content

Conversation

@user202729
Copy link

I just come across some parts of the code that can be improved:

  • the return values of urisToFilenames and FilenamesToUri are used nowhere, so I just delete it and declare the function works by mutating the object.
  • move the declaration of projectsBasePath up to deduplicate some code.
  • Reading the source code, reader and writer only need onMessage, onError, onClose and send respectively.
  • !message.method === 'textDocument/definition' evaluates as (!message.method) === 'textDocument/definition', the left hand side is always a boolean and never equal to a string. So that line is in fact useless.
  • since that translation is useless, I conclude the reverse translation is also useless.

That said, I haven't tested these locally.

@joneugster
Copy link
Collaborator

joneugster commented Oct 13, 2025

Isn't filenamesToURI modifying the object it's applied to in-place?

Number 4 looks like a pretty obvious typo to me, deleting it seems the wrong approach. Although, it would be worth to properly inverstigate why this typo doesn't break more. Anyways I think the necessity to do fileNamesToUri only arises in minor details (like Lake options not loading properly, etc. ) and in some specific settings (i.e. Windows user, or specific setup), so it's not unlikely that it wouldn't be noticed in a standard use case.

@user202729 user202729 marked this pull request as draft October 14, 2025 00:17
@user202729
Copy link
Author

Isn't filenamesToURI modifying the object it's applied to in-place?

That's what I mean, it modifies in-place and only used as such.

Although, it would be worth to properly inverstigate why this typo doesn't break more.

Agree. (and also worth putting it in the documentation. Alternatively if e.g. Windows is the culprit, then do the replacement on Windows only suffices (to avoid surprising behavior I guess.)

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