Skip to content

Conversation

@tmat
Copy link
Member

@tmat tmat commented Sep 1, 2021

Read global options (options not serialized to the solution snapshot) directly using IGlobalOptionService rather then via Solution.Options or Workspace.Options.

@ghost ghost added the Area-IDE label Sep 1, 2021
@tmat tmat force-pushed the RemoteHostOptions branch from 62a66a6 to f5de977 Compare September 2, 2021 18:33
@tmat tmat force-pushed the RemoteHostOptions branch from 4ed616e to ca74658 Compare September 3, 2021 22:52
@tmat tmat marked this pull request as ready for review September 3, 2021 23:49
@tmat tmat requested a review from a team as a code owner September 3, 2021 23:49
@tmat
Copy link
Member Author

tmat commented Sep 3, 2021

@dibarbet @dotnet/roslyn-ide PTAL

Copy link
Member

@dibarbet dibarbet left a comment

Choose a reason for hiding this comment

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

had a general overall question when reading through this PR - How do I know when an option should be retrieved (or set) from the IGlobalOptionService vs. the solution options? For example, why are the NavigationOptions solution options and not global ones?

Clearly feature flags are generally global options. Is everything else a solution option? And will I get an error if I try and use a solution option to query the IGlobalOptionService?

Otherwise PR looks good

@tmat
Copy link
Member Author

tmat commented Sep 4, 2021

had a general overall question when reading through this PR - How do I know when an option should be retrieved (or set) from the IGlobalOptionService vs. the solution options? For example, why are the NavigationOptions solution options and not global ones?

Clearly feature flags are generally global options. Is everything else a solution option? And will I get an error if I try and use a solution option to query the IGlobalOptionService?

Otherwise PR looks good

That's currently a bit of a problem to see. The solution option setter serializes options whose provider is defined in Workspaces or Features layer. Those options are solution options - they are serialized over to OOP. Other options are global options, which are not available in OOP.

I'm thinking of introducing marker interfaces IGlobalOptionProvider/ISolutionOptionProvider to make it more explicit.

Hopefully, once I'm done with the refactorings it will be clearer. Currently the system is very complex.

@tmat tmat merged commit 4fada24 into dotnet:main Sep 7, 2021
@ghost ghost added this to the Next milestone Sep 7, 2021
@tmat tmat deleted the RemoteHostOptions branch September 7, 2021 16:43
@dibarbet
Copy link
Member

dibarbet commented Sep 7, 2021

I'm thinking of introducing marker interfaces IGlobalOptionProvider/ISolutionOptionProvider to make it more explicit.

I think that would be good. Is it also possible to mark an Option or Option2 as being global or solution based, and get a compile error if I try and retrieve a global option from the solution for example?

@tmat
Copy link
Member Author

tmat commented Sep 7, 2021

I think that would be good. Is it also possible to mark an Option or Option2 as being global or solution based, and get a compile error if I try and retrieve a global option from the solution for example?

Unfortunately, not as it is designed today. Options do not know which provider owns them. In theory they can be owned by multiple.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants