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

fix: repair volar windows version and python windows paths #1141

Merged
merged 5 commits into from
Jan 3, 2025

Conversation

Yuki61803
Copy link
Contributor

  1. There was an error in json which caused a parsing error in python.
  2. There was a error with using dynamically returned paths for Windows:
    SyntaxError: (unicode error) 'unicodeescape' codec can't decode bytes in position 2-3: truncated \UXXXXXXXX escape.
  3. There was an error with identifying the LSP server as an object due to the same names of the Volar.

Fix tested on Windows 10.

core/utils.py Outdated
Comment on lines 495 to 498
if get_os_name() == "windows":
user_emacs_dir = get_emacs_func_result("get-user-emacs-directory").replace("/", "\\")
else:
user_emacs_dir = get_emacs_func_result("get-user-emacs-directory")
user_emacs_dir = repr(get_emacs_func_result("get-user-emacs-directory")).strip("'")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this patch for Windows platform, why repr on non-Windows branch?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok i just saw that unicode parsing only works via escape sequences.
It's better to remove this.

lsp-bridge.el Outdated
Comment on lines 455 to 457
(if (string-match "windows" (symbol-name system-type))
(setf (cdr (assoc '("vue") lsp-bridge-multi-lang-server-extension-list)) "volar_emmet_windows")
)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setf should use in defcustom

Copy link
Contributor Author

@Yuki61803 Yuki61803 Jan 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you write below how to do this right way?
I don't quite understand.
I initially wanted to do this check inside defcustom but it made the code unreadable.

@@ -1,5 +1,5 @@
{
"name": "volar",
"name": "volar_windows",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name should not change.

We just need change volar_windows.json name, don't need change "name"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes another bug with server not found.
Lsp server gets name volar and program searches it by volar_windows.
So we need to change the name.

Comment on lines 1 to 22
{
"completion": ["volar_windows", "emmet-ls"],
"completion_item_resolve": ["volar_windows", "emmet-ls"],
"diagnostics": ["volar_windows", "emmet-ls"],
"code_action": ["volar_windows", "emmet-ls"],
"execute_command": ["volar_windows", "emmet-ls"],
"find_define": "volar_windows",
"find_type_define": "volar_windows",
"find_implementation": "volar_windows",
"find_references": "volar_windows",
"peek_find_definition": "volar_windows",
"peek_find_references": "volar_windows",
"formatting": "volar_windows",
"hover": "volar_windows",
"signature_help": "volar_windows",
"prepare_rename": "volar_windows",
"rename": "volar_windows",
"document_symbol": "volar_windows",
"workspace_symbol": "volar_windows",
"semantic_tokens": "volar_windows",
"inlay_hint": "volar_windows"
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

volar_windows.json 's name should use "volar",

above also should change to "volar".

lsp-bridge will choose volar_windows.json when it run on Windows platform

Copy link
Contributor Author

@Yuki61803 Yuki61803 Jan 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes another bug that I don't remember.
I'll write it tomorrow when i reproduce it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Infinite creation of connections until emacs crashes.

@manateelazycat
Copy link
Owner

@Yuki61803 I have push fb0e319

You don't need change option lsp-bridge-multi-lang-server-extension-list, lsp-bridge will use volar_emmet_windows.json instead volar_emmet.json on Windows platform.

@Yuki61803
Copy link
Contributor Author

Yuki61803 commented Jan 1, 2025

@Yuki61803 I have push fb0e319

You don't need change option lsp-bridge-multi-lang-server-extension-list, lsp-bridge will use volar_emmet_windows.json instead volar_emmet.json on Windows platform.

Ok. I'll check this push tomorrow.

@Yuki61803
Copy link
Contributor Author

Yuki61803 commented Jan 3, 2025

As you said, everything is correct.
I was busy yesterday. Reproduced today with the push mentioned above.
In the end, only three lines will change to fix issue with windows volar.

@manateelazycat manateelazycat merged commit a8e5d67 into manateelazycat:master Jan 3, 2025
1 check passed
@manateelazycat
Copy link
Owner

@Yuki61803 Thank you very much, good patch! ;)

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