Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add path-mapping function #2579

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open

add path-mapping function #2579

wants to merge 21 commits into from

Conversation

95833
Copy link

@95833 95833 commented Dec 12, 2024

No description provided.

@95833 95833 closed this Dec 21, 2024
@rwols
Copy link
Member

rwols commented Dec 22, 2024

Why did you close this? It seems useful. Note that it can be hard for maintainers to make time to review every pull request. I will re-open this just in case.

@rwols rwols reopened this Dec 22, 2024
@95833
Copy link
Author

95833 commented Dec 22, 2024

I am fixing some bugs, so I have temporarily closed it.

@rwols rwols marked this pull request as draft December 22, 2024 10:04
@rwols
Copy link
Member

rwols commented Dec 22, 2024

I have converted the PR status to "draft" then. You can remove the "draft" status when you think it's ready.

@95833
Copy link
Author

95833 commented Dec 22, 2024

Alright, it should now be usable to some extent. At least on the surface, it seems that no new trouble have been introduced.

@95833 95833 marked this pull request as ready for review December 22, 2024 12:18
@95833
Copy link
Author

95833 commented Dec 22, 2024

There are still some bugs, but I am unable to fix them. for example:
1
When I click on it, it opens a path that is not the displayed one, but rather a path that has not been mapped. After I tracked down the _diagnostics_async function in the documents.py file and added a print(sb.diagnostics) statement as shown in the image below,
3
The following content was printed out.
2
It can be seen that the initial diagnostic path has been successfully mapped, but then it prints out paths that have not been mapped. I don't know how to fix it.

@jwortmann
Copy link
Member

Could you describe the use case for this; there isn't any linked issue or explanation in the opening comment, and I don't have any intuition just from the config description

Map the paths of URIs in the data exchanged between the server and the client.

why I would want to map paths like that. Apparently it has something to do with WSL. I think from the regular contributors I'm the only Windows user, but I've never really used WSL, so if this requires a certain setup then a step by step description would be useful if we want to test this PR.

Ideally this PR would have a test with something that currently doesn't work, but will be fixed by this PR (if this is applicable here).

@95833
Copy link
Author

95833 commented Dec 23, 2024

I use it in the WSL environment, but it is a general feature. clangd also has a path mapping option, and their functions should be consistent. However, clangd also has bugs and cannot meet my needs. @rwols Should be able to provide more explanation about this feature.

In my use case, I code using an editor on Windows and compile it with the toolchain and libraries in WSL. Since the project is compiled on Linux, the paths in the compile_commands used by clangd are all Linux paths. To correctly navigate to files, the LSP needs to map the Linux paths provided by clangd to Windows paths.

It's not the most natural way to implement it, but for someone like me who is not familiar with the LSP source code, it's the simplest way to do it. Even so, a strange bug still occurred.

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.

3 participants