Skip to content

Clean up Solution Explorer context menus - #988

Merged
tmeschter merged 8 commits into
dotnet:masterfrom
tmeschter:Fix619
Mar 5, 2015
Merged

Clean up Solution Explorer context menus#988
tmeschter merged 8 commits into
dotnet:masterfrom
tmeschter:Fix619

Conversation

@tmeschter

Copy link
Copy Markdown
Contributor

Fixes #619.

I suggest looking at the individual commits one at a time, from top to bottom. Each one is fairly small.

The major work here is to move the context menu items for setting a diagnostic's severity (Error, Warning, etc.) into a sub menu and making it clear that the check marks represent the severity in the rule set specifically, and clicking the menu items will update the rule set file.

Along the way I encountered other bugs I needed to fix to ensure the proper experience here.

This makes it a little more clear which .ruleset file is going to be
opened, and brings the text in line with other menu items such as "Set
as Active Rule Set".
This commit moves the menu items to change a diagnostic's severity
(Error, Warning, Info, etc.) off of the context menu and into a sub menu
under "Set Rule Set Severity". This reduces clutter and makes it clearer
what is going to be changed by selecting one of these.
We only want to show the "Open Help Link" menu item when a single
diagnostic is selected in Solution Explorer. However, we weren't
updating the visibility correctly when the set of selected diagnostics
changed.
Right now we update a bunch of state within our Solution Explorer
context menus on every change to the set of selected items in the
Solution Explorer. This state includes things like which items are
visible, which are enabled, and which are checked. In most cases it
would be better to simply wait until a context menu is about to be
displayed, and then go update the item state.

The big change here is that ContextMenuController now updates the menu
item state before showing the menu; this requires that the
AnalyzersCommandHandler create the ContextMenuController instances since
it owns the menu items.

Most of this change is simply about piping the ContextMenuControllers
from the AnalyzersCommandHandler down to the individual
AnalyzersFolderItems/AnalyzerItems/DiagnosticsItems. I also had to add
an IAnalyzerCommandHandler interface so that I could create a no-op
implementation for the unit tests.
`DiagnosticItem.BrowseObject` has a property, `DiagnosticItem`, that is
used internally and is not meant to show up as an item in the Properties
window. This change suppresses it.
When you right-click on a diagnostic in the Solution Explorer and open
the "Set Rule Set Severity" sub menu, the check marks actually show you
the effective severity of the rule. This takes into account not just
what is in the rule set file, but also the default severity and any
relevant project options.

This is misleading for a couple of reasons. For one thing, we may show a
checkmark on a severity even though the rule doesn't appear in the rule
set file at all. Worse, choosing a different severity may update the
rule set but leave the checkmark where it is (because some other setting
overrides the rule set) and quite likely confusing the user.

This change updates the checkmarks to truly show what is in the rule set
file, and only what is in the rule set file.
@tmeschter

Copy link
Copy Markdown
Contributor Author

@srivatsn @jmarolf @shyamnamboodiripad @mavasani @heejaechang @JohnHamby Could you take a look, please?

@shyamnamboodiripad

Copy link
Copy Markdown
Contributor

👍 except for above comment

@jmarolf

jmarolf commented Mar 4, 2015

Copy link
Copy Markdown
Contributor

👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

return !items.Skip(1).Any(); ?

doesn't matter if it is expected to have small number of items though..

@heejaechang

Copy link
Copy Markdown
Contributor

👍 my comments are all minor. you can ignore them if you don't feel like changing.

If the user selects a bunch of diagnostics with differing severities in
Solution Explorer, right-clicks,  and opens the "Set Rule Set Severity"
submenu they will see a check mark next to every applicable severity.
E.g., if one rule is an error and one is a warning, then both Error and
Warn will be checked.

The general feeling is that this will be confusing to users. Instead,
only show a check mark when all the selected rules share the same
severity.
@tmeschter

Copy link
Copy Markdown
Contributor Author

@shyamnamboodiripad @srivatsn My latest commit updates the check mark behavior. If the build passes, I'm going to merge in the pull request.

@srivatsn

srivatsn commented Mar 5, 2015

Copy link
Copy Markdown
Contributor

👍

tmeschter added a commit that referenced this pull request Mar 5, 2015
Clean up Solution Explorer context menus
@tmeschter
tmeschter merged commit 389960d into dotnet:master Mar 5, 2015
dibarbet pushed a commit that referenced this pull request Aug 18, 2026
Use weighted match to allow expected generator outputs in any order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enabling rules from solution explorer doesn't do anything if there's a nowarn in the project

7 participants