Skip to content

sharing config between client and server#432

Merged
cristianoc merged 5 commits into
masterfrom
shared-configuration
May 30, 2022
Merged

sharing config between client and server#432
cristianoc merged 5 commits into
masterfrom
shared-configuration

Conversation

@zth

@zth zth commented May 28, 2022

Copy link
Copy Markdown
Member

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

@cristianoc cristianoc left a comment

Copy link
Copy Markdown
Collaborator

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.

Comment thread server/src/server.ts
Comment thread server/src/server.ts
Comment thread server/src/server.ts Outdated
Comment thread server/src/server.ts Outdated
@zth

zth commented May 30, 2022

Copy link
Copy Markdown
Member Author

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
Copy Markdown
Collaborator

Thanks! Merging.

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

Copy link
Copy Markdown
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

zth commented May 30, 2022

Copy link
Copy Markdown
Member Author

You mean when restarting via the restart command recently added?

@cristianoc

Copy link
Copy Markdown
Collaborator

You mean when restarting via the restart command recently added?

yes

@cristianoc

Copy link
Copy Markdown
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

zth commented May 30, 2022

Copy link
Copy Markdown
Member Author

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

@cristianoc

Copy link
Copy Markdown
Collaborator

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

@cristianoc

Copy link
Copy Markdown
Collaborator

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

@zth

zth commented May 30, 2022

Copy link
Copy Markdown
Member Author

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

@zth

zth commented May 30, 2022

Copy link
Copy Markdown
Member Author

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