Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line highlight on search results doesn't work #106209

Open
pbannister opened this issue Sep 7, 2020 · 23 comments
Open

Line highlight on search results doesn't work #106209

pbannister opened this issue Sep 7, 2020 · 23 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member search Search widget and operation issues
Milestone

Comments

@pbannister
Copy link

pbannister commented Sep 7, 2020

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:
4. 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.)
5. Place an underline or box around the emphasized line.
6. Bold the text of the line.
7. Add pointer glyphs in left and/or right gutters. (Right could be remote from text.)
8. 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
Copy link
Member

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?

@roblourens roblourens added the info-needed Issue requires more information from poster label Sep 10, 2020
@pbannister
Copy link
Author

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
Copy link
Member

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
Copy link
Author

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 roblourens reopened this Sep 22, 2020
@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues and removed info-needed Issue requires more information from poster labels Dec 13, 2020
@roblourens roblourens added this to the December/January 2021 milestone Dec 13, 2020
@roblourens
Copy link
Member

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
Copy link
Author

pbannister commented Dec 23, 2020 via email

@rzhao271 rzhao271 added the author-verification-requested Issues potentially verifiable by issue author label Jan 26, 2021
@github-actions
Copy link

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 1a9dd75 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 connor4312 added the verified Verification succeeded label Jan 29, 2021
@connor4312
Copy link
Member

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

@connor4312 connor4312 reopened this Jan 29, 2021
@connor4312 connor4312 added verification-found Issue verification failed and removed verified Verification succeeded labels Jan 29, 2021
@roblourens
Copy link
Member

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

@roblourens
Copy link
Member

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

this._modelDisposables.add(this._model.onDidChangeDecorations((e) => {
this.clearModelListeners();
this.removeHighlightRange();
this._model = null;
}));

@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?

@roblourens roblourens modified the milestones: March 2021, April 2021 Mar 23, 2021
@alexdima
Copy link
Member

@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.

@pbannister
Copy link
Author

pbannister commented May 29, 2021

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: 054a929
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

@roblourens roblourens removed the author-verification-requested Issues potentially verifiable by issue author label Jun 4, 2021
@roblourens roblourens modified the milestones: May 2021, June 2021 Jun 4, 2021
@roblourens roblourens modified the milestones: June 2021, Backlog Jul 1, 2021
@roblourens roblourens removed the verification-found Issue verification failed label Jul 1, 2021
@alexdima
Copy link
Member

@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
Copy link
Author

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 roblourens changed the title Visual hierarchy in display of search match. Line highlight on search results doesn't work Dec 13, 2022
@andreamah andreamah assigned andreamah and unassigned roblourens Dec 15, 2022
@roblourens
Copy link
Member

Just so happens that @andreamah just did this #106209 (comment) 😁

@akbyrd
Copy link
Contributor

akbyrd commented Feb 13, 2023

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.

@andreamah andreamah modified the milestones: Backlog, On Deck Feb 13, 2023
@pbannister
Copy link
Author

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

@andreamah
Copy link
Contributor

image

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

@andreamah andreamah modified the milestones: On Deck, December 2023 Dec 5, 2023
@akbyrd
Copy link
Contributor

akbyrd commented Dec 5, 2023

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
Copy link
Contributor

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

@andreamah andreamah reopened this Dec 6, 2023
@andreamah andreamah modified the milestones: December 2023, On Deck Dec 6, 2023
@andreamah andreamah added the confirmed Issue has been confirmed by VS Code Team member label Dec 6, 2023
@pbannister
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug confirmed Issue has been confirmed by VS Code Team member search Search widget and operation issues
Projects
None yet
Development

No branches or pull requests

8 participants
@pbannister @roblourens @akbyrd @connor4312 @alexdima @rzhao271 @andreamah and others