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

sharing config between client and server #432

Merged
merged 5 commits into from
May 30, 2022
Merged

Conversation

zth
Copy link
Collaborator

@zth zth commented May 28, 2022

Fixes #23

This introduces configuration in the extension, periodically synced to the language server for use. We'll probably need to revisit this if we get more intricate configuration needs later on, but for now this is simple enough and will do for the cases we currently have.

@zth zth requested a review from cristianoc May 29, 2022 16:45
Copy link
Collaborator

@cristianoc cristianoc left a comment

Choose a reason for hiding this comment

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

How about adding the config to the example-project as an example.
Also, should the README be updated.

@zth
Copy link
Collaborator Author

zth commented May 30, 2022

Updated the readme. Can't set up the configuration in the example project though, because VSCode doesn't save settings in the repo by default.

@zth zth requested a review from cristianoc May 30, 2022 06:07
@cristianoc cristianoc merged commit a3f549d into master May 30, 2022
@cristianoc
Copy link
Collaborator

Thanks! Merging.

@cristianoc cristianoc deleted the shared-configuration branch May 30, 2022 06:55
@cristianoc
Copy link
Collaborator

Nit: does not look like changing the setting and restarting the server picks them up.
Reloading the window does.
At least in debug mode. Haven't tried if it's the same when installing the .vsix file

@zth
Copy link
Collaborator Author

zth commented May 30, 2022

You mean when restarting via the restart command recently added?

@cristianoc
Copy link
Collaborator

You mean when restarting via the restart command recently added?

yes

@cristianoc
Copy link
Collaborator

Or I guess the more general question is: why polling. If one needs to restart anyway.
Maybe having to restart is not bad, and one can make everything simpler and remove the polling logic.

@zth
Copy link
Collaborator Author

zth commented May 30, 2022

Hmm, I don't need to restart anything, polling works fine for me as I tested. How are you testing it?

@cristianoc
Copy link
Collaborator

What I've tried is: change the setting, then ask the server to restart.

@cristianoc
Copy link
Collaborator

Don't know what else to test without restarting the entire editor.

@zth
Copy link
Collaborator Author

zth commented May 30, 2022

Ahh, I think I know the issue. I'll fix it.

@zth
Copy link
Collaborator Author

zth commented May 30, 2022

Done here: #438

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.

Prevent the "Start a build for this project to get the freshest data?" notification from spamming
2 participants