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

Is it normal that we need to restart omnisharp when we change formattingOptions? #1080

Closed
Thaina opened this issue Dec 23, 2016 · 10 comments
Closed

Comments

@Thaina
Copy link
Contributor

Thaina commented Dec 23, 2016

Today I just start using omnisharp.json in vscode. And it not always format when I change that file. It seem like it need to restart omnisharp every times I change option to make it work

Is this is expected behaviour? Should it always inject formattingoption everytime it changed?

@DustinCampbell
Copy link
Member

As far as I can tell, OmniSharp loads the options once at start up. However, you're right that this isn't a great experience. Changing this file should update the options on the fly.

@filipw
Copy link
Contributor

filipw commented Dec 23, 2016

I agree - it should be modifiable in real time.
There is actually already an issue to track this - OmniSharp/omnisharp-roslyn#366

It should be fairly easy to implement.

@DustinCampbell
Copy link
Member

FWIW, I wouldn't support that by sending the options with every request as mentioned as a suggested solution in OmniSharp/omnisharp-roslyn#366. Instead, OmniSharp should be watching the omnisharp.json and update the options its using when that file changes.

Anyway, we can track that issue. Closing this as a dupe of OmniSharp/omnisharp-roslyn#366.

@filipw
Copy link
Contributor

filipw commented Dec 23, 2016

yes I wouldn't do it per request too.

However it might be useful to have a route which you can use to modify the options omnisharp started with.

Not sure if VS Code exposes relevant events for that at the moment, but this way i.e. if the user changes the value of editor.tabSize, the extension could use the options route to modify the corresponding value in the running OmniSharp process.

@Thaina
Copy link
Contributor Author

Thaina commented Dec 23, 2016

I'm not sure that issue OmniSharp/omnisharp-roslyn#366 keep track about the modification of formatting file after omnisharp was started

It has been talk about inherit the setting from config.json. But what happen if config.json was changed after that?

And vscode has expose some event related to file changed. So I think it still in scope of extension (this repo) to pass the notification from vscode to omnisharp for updating the options

@DustinCampbell
Copy link
Member

@filipw: Totally agreed.

@Thaina: The bulk of the work will be in the OmniSharp server with a small amount of work to handle the specific scenario @filipw mentioned. So, I'd rather track it over in omnisharp-roslyn -- especially since the C# extension in VS Code knows nothing about omnisharp.json. I'll update OmniSharp/omnisharp-roslyn#366 to better track this issue.

@Thaina
Copy link
Contributor Author

Thaina commented Dec 23, 2016

Talk about omnisharp.json, if it is the base config file of omnisharp. I wish that we should add it into validator contribution point for omnisharp-vscode

Actually I was trying to make PR just today. But I can't find its schema

@DustinCampbell
Copy link
Member

The "schema" is generated dynamically by the options that OmniSharp expects to be there. I agree that having something on schemastore.org that we regularly update would be good.

@Thaina
Copy link
Contributor Author

Thaina commented Dec 23, 2016

If it could generate dynamically then I think it should always generate automatically into omnisharp repo and let other extension reference to it

@DustinCampbell
Copy link
Member

Yup. I agree. We're taking contributions btw 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants