fix paths on Windows (or at least attempt to)#75
Conversation
|
Changes in main.lua -> made (manually) |
|
Are you sure you applied the patch correctly? It should be impossible for it to log You can try the changes in this PR by running the following commands (assuming you've cloned the repository to cd ~/.config/micro/plug/mlsp
git fetch origin windows-path-fix:windows-path-fix
git restore --source=windows-path-fix -- main.lua |
|
Are you sure you applied the patch correctly? -> Yes (edited main.lua with micro and double checked the changes.) Now with: log.txt seems to be the same except for: |
|
It must still be using the old version of main.lua somehow – I've checked multiple times and code that could create a file uri with only two slashes doesn't exist any more. Is it possible that you have the plugin in two different places, only one of which is the configuration directory for micro? Note that the path in the |
|
The cause of test problems was that i have the initial Micro does not give an error after removing Then only with the modified MLSP exits and has to be restarted with log.txt:
|
That makes sense, micro doesn't care about the filenames, it just executes all .lua files in the plugin directory (in alphabetical order if I recall correctly). I think the new error is because in Windows paths are case-insensitive (sigh..), so we should treat handle https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri |
|
I checked out this pr and tried to run ruff but still got exit status 2. |
|
@bjornasm that looks like yet another different Windows path-handling bug because I don't see the |
|
How do you log the Lua errors? I tried to hardcode the path inside the function, but it seems like the crash is happening before that: Produces: |
You can use the log function: Lines 636 to 638 in 8785474
The workspace URL is set on startup in Lines 287 to 288 in 8785474 edit: fixed link to point to code from this PR |
|
A bit more debugging to do it seems, but looks like I am on the right way. edit: Got it to work, no errors and the logs look fine. Let me know how I can contribute my code. |
|
A new PR to be merged to this branch ( |
|
I am working on this here https://github.com/bjornasm/mlsp/tree/windows-path-fix-debug and doing the windows tests from #74 (comment) This test fails - but I am unsure that its intended to change the drive letter to be upper case? Windows itself doesn't require this afaik, but this might be an issue with the LSP's? Other than that all (windows) tests pass. |
|
From https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#uri
So it doesn't necessarily need to be specifically capitalized but it must map to the exact key used in |
Interesting, thank you. Initially I think that means we have to pass it Is it possible to allways put all drives in upper case to make sure? |
Yes, I think that would be a good idea. Maybe we could even uppercase the entire path (both file URIs from the server and buf.AbsPath from micro) on Windows? |
Thank you for your input, I'll try that. Sorry for the late reply! Edit: I was able to implement this and it doesn't crash, but it doesn't format any code either weirdly. Tried with pylsp and ruff. I also get "no hover results" etc. Seems like its working on an empty file instead of the one I have open maybe.. Do you have some quick debugging to show the file its working on? Edit2: Found it, got to debug a bit more, will update with a new comment when I have made progress. |
|
Ok, I got it to work without errors. I couldn't test the LSP with python as I gotr no errors but also no results on format, hover etc, but I guess there must be something wrong in how I have set up the LSP. I did a quick test with markdown and that works, so I have to go back to see what the problem was with my python, but I guess that is a different issue. Edit: Got it up and running for python lsp's as well, and you tests are passing. |
|
I created a pull request #77 from your changes, let's continue the discussion there. |
This may or may not fix #74 (need someone using Windows to test)
closes #74