Skip to content

Ensure that TodoCommentOptions are serialized to OOP#44873

Merged
mavasani merged 1 commit intodotnet:masterfrom
mavasani:Issue43788
Jun 5, 2020
Merged

Ensure that TodoCommentOptions are serialized to OOP#44873
mavasani merged 1 commit intodotnet:masterfrom
mavasani:Issue43788

Conversation

@mavasani
Copy link
Contributor

@mavasani mavasani commented Jun 5, 2020

Fixes #43788
TodoCommentOptions and TodoCommentOptionsProvider were defined in EditorFeatures layer and we only serialize the options defined in Workspaces and Features layer to the OOP side:

// We only consider the options defined in the the DefaultAssemblies (Workspaces and Features) as serializable.
// This is due to the fact that other layers above are VS specific and do not execute in OOP.
var provider = lazyProviderAndMetadata.Value;
if (!MefHostServices.IsDefaultAssembly(provider.GetType().Assembly))
{
continue;
}

In future, we either need to add support to serialize all changed options to OOP (could be expensive) or somehow enforce that all the OOP related options are defined in Workspaces and Features layer.

Fixes dotnet#43788
TodoCommentOptions and TodoCommentOptionsProvider were defined in EditorFeatures layer and we only serialize the options defined in Workspaces and Features layer to the OOP side: https://github.com/dotnet/roslyn/blob/287a25a51324f252e61b6f2186e7df60c8a6161b/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs#L79-L85

In future, we either need to add support to serialize all changed options to OOP (could be expensive) or somehow enforce that all the OOP related options are defined in Workspaces and Features layer.
@mavasani mavasani added this to the 16.7 milestone Jun 5, 2020
@mavasani mavasani requested review from a team and CyrusNajmabadi June 5, 2020 00:22
Copy link
Contributor

@jmarolf jmarolf left a comment

Choose a reason for hiding this comment

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

:shipit:

@CyrusNajmabadi
Copy link
Contributor

Were you able to validate if this fixes things?

@mavasani
Copy link
Contributor Author

mavasani commented Jun 5, 2020

@CyrusNajmabadi Yes, the repro in the issue is fixed with this change.

@mavasani mavasani merged commit c552d52 into dotnet:master Jun 5, 2020
@ghost ghost modified the milestones: 16.7, Next Jun 5, 2020
@mavasani mavasani deleted the Issue43788 branch June 5, 2020 09:09
@RikkiGibson RikkiGibson modified the milestones: Next, 16.7.P3 Jun 8, 2020
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.

Custom tokens not displayed in task list

4 participants