microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
163.61k stars 29.03k forks source link

Line highlight on search results doesn't work #106209

Open pbannister opened 4 years ago

pbannister commented 4 years ago

There is a lack of any sort of visual hierarchy in walking through multi-file search matches.

  1. Run a search over a largish set of sources. (Ctrl-Shift-F)
  2. Click through the matches in the SEARCH panel, and try to find the matching source editor line.

Put differently, when I click on a match in the SEARCH panel, my eye has no guide to find the matched text. When clicking through matches in the SEARCH panel, I expect some sort of visual hierarchy to help my eye navigate to the matching line and section. End up scanning the page to find the match, failing, and ... saying rude things. (This is on a 32" 4K monitor.)

What I expect is some sort of visual hierarchy. First, I need to find the line in question (vertical). Then I need to find the selected text (horizontal).

The vertical cue is lacking.

There are many ways to solve the vertical cue. Past-used alternatives of which I am not fond.

  1. Hard-force the match to center of screen. (Though on large screens, you need a cue for center.)
  2. Hard-force the match to top line of screen.
  3. Hard-force the match to fixed N-lines down from top of screen.

Better alternatives:

  1. Highlight the line by changing the background color. Needs to contrast well with syntax coloring. (Ancient choice was to XOR the line colors - which would be better than present.)
  2. Place an underline or box around the emphasized line.
  3. Bold the text of the line.
  4. Add pointer glyphs in left and/or right gutters. (Right could be remote from text.)
  5. Apply temporary highlight to the matched line, when selected from SEARCH.

A completionist would add custom setting for each specific font/weight/color/background for aspect of line and selection of match. While "better" in some sense, this is not fun to configure.

Did look at the "workbench.colorCustomizations", "editor.findMatchBackground", "findMatchHighlightBackground", but not obviously useful. Inclined to think the code to lacks any notion of a visual hierarchy in presenting search results.

roblourens commented 4 years ago

We do put the match in the center of the screen when it is possible to do so. We also change the background color of the selected match. Some people like to set editor.findMatchBorder to put a bright colored border around the selected match, but I wouldn't do that by default because vscode prefers a minimalist color scheme. Can you explain more about why this isn't sufficient?

pbannister commented 4 years ago

Search should highlight matches the same as Find (in a file).

First I have a larger monitor (31" 4K) using the "Dark+(default dark)" color scheme.

Back in ancient days, on smaller screens, with monochrome text - a blinking cursor or simple highlight stood out fairly well. Simply centering the screen around the match was sufficient. The eye could find the match with little trouble.

The reason I am not over-fond (as mentioned above) of the centering alone (as cue) is this works poorly on large visually busy screens. Syntax highlighting means the display is very "busy" visually. The matched area is quite small on large screens. Not to mention the "center" will be somewhere in the top half of the screen, if near top of file. So centering is not enough.

The search match is highlighted, but not obvious on a large visually busy screen. Think of this like cross-hairs. When you have a small target in a large space, you need a visual aid. The editor does supply other visual cues for vertical place. When you set the focus, the editor places a faint top/bottom border on the line (good). When you do "Find" within a file, the editor places a faint background highlight on the entire line (good).

Search across files does rather less.

roblourens commented 4 years ago

Matching the background highlight on the line that find in the editor does would be good. @rebornix what is controlling that line highlight? It's a different color than the normal line hightlight.

pbannister commented 4 years ago

I added more information the same day, but saw no way to change the "needs information" label. Presumed @roblourens or the like had the needed privilege. The auto-close seems in error.

roblourens commented 3 years ago

We actually intended to have the line highlights, but this stopped working some time ago. I will reinstate that. Thanks for the suggestions, this is the only change I plan to make here right now.

pbannister commented 3 years ago

Cool. Looking forward to the released fix. (No pun intended.)

Bit by polymorphism, looks like. :)

On Sun, Dec 13, 2020 at 12:04 PM Rob Lourens notifications@github.com wrote:

Closed #106209 https://github.com/microsoft/vscode/issues/106209 via 873f23d https://github.com/microsoft/vscode/commit/873f23dc4cf1084e285b32349ee73fcd433266c2 .

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/microsoft/vscode/issues/106209#event-4106386760, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABUEJYGRHWRDVQI5UMQDITSUUM45ANCNFSM4Q5PJQAQ .

github-actions[bot] commented 3 years ago

This bug has been fixed in to the latest release of VS Code Insiders!

@pbannister, you can help us out by confirming things are working as expected in the latest Insiders release. If things look good, please leave a comment with the text /verified to let us know. If not, please ensure you're on version 1a9dd758530782ee4bb09a15bdc484f3dcb9b5a3 of Insiders (today's or later - you can use Help: About in the command palette to check), and leave a comment letting us know what isn't working as expected.

Happy Coding!

connor4312 commented 3 years ago

When clicking from the search view, I do see a very brief highlight flash but it doesn't stick https://memes.peet.io/img/21-01-31ea768f-fa8f-4af0-999e-337b03bc5346.mp4

roblourens commented 3 years ago

I think it's getting cleared when the gitlens decoration shows up, I'll figure out what to do about that...

roblourens commented 3 years ago

It's cleared when any decoration on the model changes.

https://github.com/microsoft/vscode/blob/cdbc22a9cbdad032fee5eb97b36c911e7134c2bb/src/vs/workbench/contrib/search/common/searchModel.ts#L1256-L1260

@alexdima you added this, I guess we do this because we don't know whether this decoration will conflict with other decorations? Is anything really bad going to happen if we don't remove it?

alexdima commented 3 years ago

@roblourens IIRC I think this was a layering problem. When it was initially written, the search component was using the RangeHighlightDecorations from here. That one used to be in /common/. But I had to move the editor out of /common/ because that didn't make any sense and was causing ridiculous amount of pain for developing the editor... The editor widget only loads in a browser environment, it can never load in nodejs or in a web worker, etc. So all code that sits in /common/ and uses an editor actually also always executes in /browser/. Long story short, I could not just move the entire search model over to /browser/ (maybe you would want to do that since that only executes in a browser environment), so the result is the RangeHighlightDecorations from searchModel.ts which works with a text model (IModelService) and not with an editor (IEditorService). In the meantime, the RangeHighlightDecorations from here probably diverged.

roblourens commented 3 years ago

Thanks. The difference here is that the other one hides the decorations on onDidChangeCursorPosition and I'm guessing that onDidChangeDecorations was used to hit the same case with the text model. But it would be better to try to get rid of that and figure out how to hide the search decoration only when the cursor moves.

alexdima commented 3 years ago

But it would be better to try to get rid of that and figure out how to hide the search decoration only when the cursor moves.

:+1: The problem is that the cursor positions is an editor concept, so that code would need to be in /browser/. The alternative would be to do some refactorings to be able to plug in a decorations manager that can live in /browser/ to the searchModel that can continue staying in /common/.

pbannister commented 3 years ago

I see the "author-verification-requested" label, but not sure what that means. Did a multi-file search - and I can see the match highlighting attempted but buggy.

On the second-and-later matches within the same file, the match line does have a highlighted background across the width of the text editor panel.

On the first match within a file, I see that same highlight for a fraction of a second, then it disappears. So ... I can verify there is an attempt at getting this right. :)

Oh. As a work-around, click on the search-results item twice. The second time the highlight stays. :)

Did try disabling all other extensions, and enabling just cpptools. Same behavior. This is with cpptools 1.4.0 and ... Version: 1.56.2 Commit: 054a9295330880ed74ceaedda236253b4f39a335 Date: 2021-05-12T16:45:26.313Z Electron: 12.0.4 Chrome: 89.0.4389.114 Node.js: 14.16.0 V8: 8.9.255.24-electron.0 OS: Linux x64 5.4.0-73-generic

alexdima commented 3 years ago

@roblourens Maybe a straight-forward way to tackle this would be to move searchModel to /browser/. That should work unless this file needs to be executed in the main process, in a web worker or in the extension host.

pbannister commented 2 years ago

Checking on this, a year(!) later. Behavior in 1.68.1 is the same as 1.56.2 (May 2021). You still have to click twice to get the find-match-line highlighted. Even then the current/match-line background is too subtle for my eyes/monitor. Looking at alternatives.

Related issue: #34105

Topic on StackOverflow: https://stackoverflow.com/questions/35926381/change-highlight-text-color-in-visual-studio-code

Possibly relevant theme color overrides: Settings for Theme Colors

My experiments in settings.json (not very successful):

    // Overrides colors from the currently selected color theme.
    "workbench.colorCustomizations": {
        // "editor.findMatchBorder": "#ff0000",
        // "editor.findMatchHighlightBorder": "#ff0000",
        // "editor.findMatchHighlightBackground": "#ff0000",
        // "editor.findMatchBackground": "#ff0000",
        "editor.lineHighlightBackground": "#333300",
        // "editor.selectionHighlightBackground": "#333300"
    },
roblourens commented 1 year ago

Just so happens that @andreamah just did this https://github.com/microsoft/vscode/issues/106209#issuecomment-931060854 😁

akbyrd commented 1 year ago

When using extensions that draw decorations this issue effectively leads to search results never being highlighted. Some extensions like cpptools seem to redraw decorations any time the editor scrolls. So for some files there's essentially never a line highlight, even when stepping through results in the same file.

Like the original author I frequently have a very hard time finding where focus was moved to in the editor when stepping through search results. I finally realized this is probably the primary culprit.

pbannister commented 1 year ago

On 1.75.1 and not seeing any improvement, but presumably On Deck means upcoming. :)

andreamah commented 10 months ago
image

From what I see now, there is a background on the line for the current result, so this should resolve this.

akbyrd commented 10 months ago

This is still broken as of

Version: 1.85.0-insider (user setup)
Commit: c46c2f120b96f1d538333b263e42d1f133a200f0

I repro'd by opening an Unreal Engine workspace, searching for FCustomPrimitiveData and stepping through results. No extensions are installed. Notice the line highlight showing up briefly then disappearing.

Still There

andreamah commented 10 months ago

Right, this only happens when there's a line decoration.

image
pbannister commented 10 months ago

Probably repeating some of the above observations. Downloaded current (1.85.0) vscode, and updated extensions.

First, the line-highlight when it does appear, is nearly imperceptible. Perhaps your eyes/screen are different. Screenshot from 2023-12-10 16-01-53

I did look for customizations, and got better current-line (not search match) highlights. (Does not address the topic, here.) Screenshot from 2023-12-10 16-13-30

    "workbench.colorCustomizations": {
        // Kinda not - only matched substring
        // "editor.findMatchBackground": "#ff0000",
        // "editor.findMatchBorder": "#ff0000",
        //
        // Not useful
        // "editor.findMatchHighlightBorder": "#666600",
        // "editor.findMatchHighlightBackground": "#444400",
        //
        // Good if theme compatible
        "editor.lineHighlightBackground": "#282844",
        "editor.lineHighlightBorder": "#282899",
        //
        // ?? comment background highlight ??
    },

Second, the search-match highlight appears within a file, and disappears when moving from file-to-file. Clicking twice restores the (faint) highlight.

Again, perhaps not new, but confirming what I see.