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

[Tools] Daedalus Language Server article #121

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

muczc1wek
Copy link
Contributor

No description provided.

Copy link
Contributor

@piotrmacha piotrmacha left a comment

Choose a reason for hiding this comment

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

One comment related to proper casing of names. The rest is fine.

2. Type `settings.json` and select the option that appears, usually labeled "Preferences: Open User Settings (JSON)".
3. Add the following lines at the end of the file (but before `}`), replacing "Windows-1250" with the appropriate code page for your language:
```json
"daedalusLanguageServer.fileEncoding": "Windows-1250",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on the code it should also allow for a lowercase windows-1250.
https://github.com/kirides/DaedalusLanguageServer/blob/f67a464ffd56fc37c932f456edddf5947b8efe1b/langserver/parseresultsmanager.go#L63

There is also the initial value of 1252 without the windows part 🤔
https://github.com/kirides/DaedalusLanguageServer/blob/f67a464ffd56fc37c932f456edddf5947b8efe1b/langserver/lsphandler.go#L64-L65

Maybe windows1250 could work if 1252 works 😆 I'm sure there is a nice error message if the encoding is unrecognized, but would be nice of Kirides to allow all variants 🥹

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So what should I change in the guide?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I didn't mark this "review" as a request for changes, you can keep it as-is.

Just an observation for you guys to maybe move upstream to kirides.
From an UX standpoint being able to just set windows1250 everywhere without the - as an exception would be better.
Don't know about DX, as perhaps using same settings between projects is better and the guide depicts some sort of standard in the "Gothic developer space".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First of all, the plugin should automatically set the encoding of daedalus. It confuse people all the time

Copy link
Contributor Author

@muczc1wek muczc1wek Jun 22, 2024

Choose a reason for hiding this comment

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

Looks like Auronen tried to do something with that: kirides/vscode-daedalus#9

@auronen auronen merged commit b6e4dbd into Gothic-Modding-Community:dev Jul 24, 2024
1 check passed
@muczc1wek muczc1wek deleted the dls branch July 25, 2024 15:13
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.

4 participants