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

Search does not respect workbench.editor.revealIfOpen #94705

Open
akbyrd opened this issue Apr 8, 2020 · 16 comments
Open

Search does not respect workbench.editor.revealIfOpen #94705

akbyrd opened this issue Apr 8, 2020 · 16 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

@akbyrd
Copy link
Contributor

akbyrd commented Apr 8, 2020

Selecting a search result will reveal an existing editor in an unfocused group even when workbench.editor.revealIfOpen is set to false.

Steps to Repro

  • Set workbench.editor.revealIfOpen to false
  • Open two editor groups
  • Focus an editor in one of the groups
  • Perform a search that will find results in the unfocused group (note that focus state is maintained while searching. The editor title color reflects this and workbench.action.focusActiveEditorGroup will focus the correct group).
  • Select the result
  • Notice it focuses the unfocused group instead of opening a new editor in the focused group
@roblourens
Copy link
Member

Old change, @sandy081 do you know why you added this for Search to override the revealIfOpen setting? I would just remove that line to respect the setting I guess, but I don't know if it's going to trigger a lot of complaints just from old behavior changing.

#9442 (comment)

Is there any reason for this to be different for search viewlet vs quickopen and explorer? Could add a new setting but that seems unnecessary.

@roblourens roblourens added bug Issue identified by VS Code Team member as probable bug search Search widget and operation issues labels Apr 11, 2020
@roblourens roblourens added this to the Backlog milestone Apr 11, 2020
@sandy081
Copy link
Member

Yeah very old change back to 1.3 (3 years back).. Revealing the opened file is expected behavior while navigating from non-explorer views. Also it shall be consistent.
Not sure if there was this setting workbench.editor.revealIfOpen at that time @bpasero ?

@bpasero
Copy link
Member

bpasero commented Apr 14, 2020

🤷

@roblourens
Copy link
Member

So a ton of places that open editors set either revealIfVisible or revealIfOpened. Even added somewhat recently: #71672

@isidorn do you happen to know why it was necessary to add that when the editor service respects the setting? Was the goal to override the setting?

If I changed this for search I think I'd want to change it everywhere. I don't use that setting so I don't really know what users expect, but it seems like the places that force overriding it are not like "open new editor" actions, it seems like if I am clicking in Search or Problems it's more likely that I would want to reveal the editor. But I don't know, and there is also this difference between revealIfOpen and ifVisible.

@isidorn
Copy link
Contributor

isidorn commented Apr 14, 2020

@roblourens I do not remember clearly to be honest. I probably pass that argument so that editor service would not open editors if they are already open.
What could happen before:
Group 1 has editor A, B and C is Visible
Group 2 has focus and has D visible.
I click on breakpoint to open editor A. I do not want it to be reopend in Group 2, but that Group 1 gets focus and it becomse visible.

@sandy081
Copy link
Member

This is the same reason @isidorn mentioned that it was added to Problems view and Search view then and want to continue it for problems view.

@bpasero
Copy link
Member

bpasero commented Apr 14, 2020

I would only set it if you really want the experience to be in this way and not allow the user to change it, otherwise do not set it. E.g. I know we set it for some editor features like goto definition to allow for a certain experience, but that is always explicit intent by the author of that feature area.

@akbyrd
Copy link
Contributor Author

akbyrd commented Apr 14, 2020

I don't use that setting so I don't really know what users expect

To add some user context here, I expected workbench.editor.revealIfOpen to be a global override that is respected almost everywhere. Definitely anything that feels like an intentional "open an editor here" operation should respect the option, imo. This would include clicking things in the explorer, clicking breakpoints, call stack, quick open, search results, and problems. I don't have as strong of an opinion on go to definition, peek definition, and stepping in the debugger. I guess I lean toward them respecting the setting for consistency and predictability, but it's not as strong of a preference.

My problem with revealing existing documents is that the focus move screws up my ability to organize editor groups. I typically have 2 groups open where each group is logically related. A common scenario is that one group is the code I'm writing and the other group is users of the code.

I frequently search for some symbol to look through all the locations it is used. I move focus to the right group before performing the search so that all results will open in that group. This leaves my 'workspace' alone (the left group when I'm actually editing) and let's me mass close the right group when I'm done with it.

The problem is that in the middle of the search results for the symbol there are going to be entries for where the symbol is defined, which is usually where I'm editing. So I step through the results, editors are being opened in the right group, where I want them, I hit a result that reveals an already opened doc in the left group, and now all subsequent results open documents on the left.

@roblourens
Copy link
Member

Thanks @akbyrd. You have a way of filing issues that sound simple at first but lead to some interesting discussion 🤔

Here's something that I didn't realize - the default of the revealIfOpen setting is false. I thought Isidor and Sandeep's changes were just to override the user's preference but actually they are just defining the default experience. I think that's a good default experience and I'd want to protect that. To do that, we'd have to add a new setting. We could have one that controls everything that isn't the file explorer/quickopen I guess.

@muxi
Copy link

muxi commented Feb 6, 2021

Hi! Is there any update on this issue? I've been searching around for solution on the same problem as @akbyrd and found this post.

FWIW, I have a very similar user context as @akbyrd mentioned here. I usually have two groups splitting the screen; the right group is the code I'm working on ("workspace") and the left group is what I need to refer to. Very often the two groups will be opening the same file, e.g. writing something that calls a function a few hundreds of lines above, so the right group will be on the line to write the function invocation and the left group shows the signature of the function.

What happens a lot is that I want to Go To Definition(<F12>) on the left group to quickly check the definition of something in another file (e.g. definition of a type in the function signature), then immediately Go Back(<Alt> + ←) to recover the previous state. In such case, even though the focus is always on the left group, VSCode makes the right group jump to the function signature after the Go Back, while I was expecting the left group to jump back to it. So now I have to manually scroll the right group a few hundreds of lines down to locate the function invocation I'm working on, and recover the left group to the function signature.

Would really appreciate a fix on this. To me this is about the only deal breaker of switching my primary ide from vim to vscode.

@greggman
Copy link
Contributor

Copying over the illustrations from closed issue

Isn't this an important usability issue? I often want to reference two locations in the same file. For example a call to method and the method's implementation. As it is, AFAIK, there is no way to do this in VSCode except to manually open 2 windows, and manually search or otherwise navigate to the desired locations. This because if you do a global find and click on the first location in search results, the activate a 2nd window and click a different result, VSCode will move the first window, not the second.

vsc

Where as what I'm used to is (could just be me) is that he last active window switches to the 2nd result

ss

The 2nd way, "move the last active window to the search result", lets you compare 2 or more results in different windows from the same file easily. Want to compare 4 locations from the same file, open 4 panes, click a pane, click a result, repeat. If you want the window with the file already in it to be the one to receive the result you just make that the active window before clicking. Conversely, the 1st you're just out of luck. You can't use the global search. You instead have to manually open the file in 4 panes and then do a local search in each pane.

I'm surprised people don't run into this more often.

@greggman
Copy link
Contributor

greggman commented Mar 3, 2022

@roblourens @akbyrd I see this issue hasn't been visited for almost 2 years. I'm really surprised as it's making it extremely frustrating to actually get work done in VSCode. Today I needed to look at a call site and the function called, both in the same file. Both are visible in global search but AFAICT It's IMPOSSIBLE in VSCode to view both results from the global search???

Is there some other solution that I'm missing?

If I submit a PR with a new setting will it be accepted?

I'm really shocked this not affecting more people so it makes me wonder what workaround people are using.

Untitled

Here's me trying to look at both the call site and the function being called. VSCode makes me lose my context (the call site) and them I'm force to jump through a bunch of hoops to find it again since there's no way to use global search to get there and local search only shows one result at a time I now have to dig through the rules, or I have to memorize the line number. Neither of these seem like what a code editor would normally do. Do you guys seriously never run into this issue? If not, what are you doing instead?

@akbyrd
Copy link
Contributor Author

akbyrd commented Mar 4, 2022

🤷 The editing experience hasn't really been the focus for years now. I've just gotten used to all my personal little workarounds until I have time to change editors.

@greggman
Copy link
Contributor

greggman commented Mar 7, 2022

The new "Drag and drop Problems and Search results" solves this issue for me in the February 2022 version 1.65 version of VSCode. You can drag a search result to any editor pane and that pane will go to that result.

@greggman
Copy link
Contributor

Actually I take that back. While being able to drag to a window helps it still is a pretty poor UX. Trying to navigate unfamiliar code I have to manually keep track of which panes have which or else always use drag, never use click. That makes clicking a bad habit because you get used to using in and then, when you need it work with multiple panes you have to change to unfamilary behavior. Making clicking act this way is arguably a pretty bad UX.

I hope the VSCode team will consider revisiting this. Or tell us how and why the never run into this. I'd expect it to come up often on a large code base like VSCode itself.

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