Add a ctrl-alt shortcut one can hold to have all inline-hints show up.#48411
Add a ctrl-alt shortcut one can hold to have all inline-hints show up.#4841121 commits merged intodotnet:masterfrom
Conversation
|
Tagging @olegtk as i want to make sure we're being a good citizen here wrt intercepting key strokes. |
fdf53c9 to
1d86d22
Compare
1d86d22 to
20f7a51
Compare
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
| private static void Toggle(Workspace workspace, bool on) | ||
| { | ||
| // No need to do anything if we're already in the requested state | ||
| var state = workspace.Options.GetOption(InlineHintsOptions.DisplayAllOverride); |
There was a problem hiding this comment.
is it possible to disable this whole processor if this option is on? And enable when it changes? This is typing code path, so ideally there should be as less work done on UI thread as possible
There was a problem hiding this comment.
i'm not certain. is there a way to disable a processor? note: i want to be careful that if the option changes that we don't get stuck with things on the screen.
Also, on every keystroke we're already doing this sort of work in other features (i.e. tracking what you type for things like rename-tracking, completion, etc.). So i'm not particularly concerned here on perf.
| private Document? GetDocument() | ||
| { | ||
| var document = | ||
| _view.BufferGraph.GetTextBuffers(b => true) | ||
| .Select(b => b.CurrentSnapshot.GetOpenDocumentInCurrentContextWithChanges()) | ||
| .WhereNotNull() | ||
| .FirstOrDefault(); | ||
|
|
||
| return document; | ||
| } |
There was a problem hiding this comment.
Do we really need to check this? I can't really come up with a case where our content type is set but we don't have a document in a way we'd really care about here. More a "less code is better" here.
There was a problem hiding this comment.
(general concern also being as this PR stands this is running on every key event and every modifier...)
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
src/EditorFeatures/Core.Wpf/InlineHints/InlineHintsKeyProcessorProvider.cs
Outdated
Show resolved
Hide resolved
|
|
||
| // We can only enter the on-state if the user has the ctrl-alt feature enabled. We can always enter the | ||
| // off state though. | ||
| on = on && _globalOptionService.GetOption(InlineHintsOptions.DisplayAllHintsWhilePressingCtrlAlt, document.Project.Language); |
There was a problem hiding this comment.
Does this need to happen before you've compared "on" to "state"?
src/EditorFeatures/Core.Wpf/InlineHints/InlineParameterNameHintsTaggerProvider.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/VisualBasic/Impl/Options/AdvancedOptionPageControl.xaml
Show resolved
Hide resolved
|
@CyrusNajmabadi things look good other than some of stuff in the key processor but I think that's it. I guess I see now we're grabbing the document because we need it for the per-language option. Should we just make the Ctrl+Alt stuff be a single option that's shared between the languages? If you've got a VB file open on one monitor and a C# one open on the other, do you want which language your focus is on to dictate? |
|
(I was originally thinking the code could be deleted but now you've got me thinking questions with no clear opinion of an answer here.) |
|
Yup, being language specific isn't beneficial here. removing that. |
|
Hello @CyrusNajmabadi! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
UI for the feature looks like:
Looks like this:
The core concept is htat while you're holding ctrl-alt, we show all hints. When you release, we go back to showing whatever hints your standard settings are for.