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

Find All input should be populated with Global Search Buffer #39609

Closed
wprater opened this issue Dec 5, 2017 · 25 comments
Closed

Find All input should be populated with Global Search Buffer #39609

wprater opened this issue Dec 5, 2017 · 25 comments
Assignees
Labels
feature-request Request for new features or functionality search Search widget and operation issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@wprater
Copy link
Contributor

wprater commented Dec 5, 2017

After this PR #35956, the find all input should also be input with the Global Search Buffer. This is how other add and editors behave in macOS that use the global search buffer.

#39585

/cc @rebornix @alexandrudima @melvin0008 @liyanage

@vscodebot vscodebot bot added the search Search widget and operation issues label Dec 5, 2017
@rebornix rebornix assigned rebornix and unassigned roblourens Dec 5, 2017
@rebornix rebornix added editor-find Editor find operations and removed search Search widget and operation issues labels Dec 5, 2017
@rebornix rebornix assigned roblourens and unassigned rebornix Dec 5, 2017
@rebornix rebornix added search Search widget and operation issues and removed editor-find Editor find operations labels Dec 5, 2017
@rebornix
Copy link
Member

rebornix commented Dec 5, 2017

The bot is smarter than me, it's about Search indeed ;(

@roblourens
Copy link
Member

When should the search viewlet get the global search buffer? Anytime it opens/cmd+shift+f is pressed?

@wprater
Copy link
Contributor Author

wprater commented Dec 6, 2017

When should the search viewlet get the global search buffer? Anytime it opens/cmd+shift+f is pressed?

@roblourens I would argue that whenever a window gains focus, it should update all the search input fields.

likewise on change events on the input fields which should be updated the global buffer and most all cocoa apps would update their search input fields on window focus.

@roblourens roblourens added this to the December 2017 milestone Dec 6, 2017
@roblourens roblourens added the feature-request Request for new features or functionality label Dec 6, 2017
@melvin0008
Copy link
Contributor

@wprater I would like to confirm the feature request.

This feature request states that similar to #35956 , we should also populate the 'find all' clipboard.

Is the above correct?

@roblourens
Copy link
Member

Right

@roblourens
Copy link
Member

If the find widget is already visible, then it doesn't take the global buffer until cmd+g/f3 is pressed. So it seems like the search viewlet shouldn't have its query replaced when it's already visible and is possibly in the middle of a search. But the search viewlet is obviously used differently and could be open when it isn't being used.

@wprater
Copy link
Contributor Author

wprater commented Dec 7, 2017

how do we handle the find all pane then? if it were not visible Id expect the global search buffer to populate all search input fields (find and find all) in all the project windows.

this is how Ive operated across many app and editors for years.

wonder if I need to write a more complex example, or do we all agree on the functionality that's standard for mac users?

@roblourens
Copy link
Member

roblourens commented Dec 7, 2017

I don't disagree that it's standard - just thinking out loud about whether the search viewlet is a different case. But trying it some more in different apps, I guess not - I think it should work as you say.

I also just can't get over the fact that I've never noticed or heard of this before, but now I can't help but notice it.

@wprater
Copy link
Contributor Author

wprater commented Dec 7, 2017

Yeah, its a really nice feature! I generally will cmd+e in Safari to put text into global search buffer, then go to the Terminal, XCode, or an editor and use cmd+g to find items in the window or use find all command.

@wprater
Copy link
Contributor Author

wprater commented Dec 21, 2017

@roblourens is this in insiders yet, as Im not seeing this fixed as of yet.

If the Find all viewlet is already open, its not being updated with the updated buffer.

@roblourens
Copy link
Member

Yes, it's only updated when the text box gets focus

@wprater
Copy link
Contributor Author

wprater commented Dec 22, 2017

This is by design? Makes going from one project to another to use Find All with the Global Search Buffer a non-starter tho?

@roblourens
Copy link
Member

How so? It works fine for me, does it not work for you?

@liyanage
Copy link

Hm it's working for me today. A few days ago I also thought that it wasn't working.

Re-reading @wprater's comment, I realize that it only updates when the editor pane has the focus. If for example a file in the sidebar is selected and the app is brought to the foreground, the search box doesn't update.

That is not intuitive, and it is not how other Mac apps work. Other apps update the search text immediately when they are brought to the foreground, and I think VSC should behave the same.

See the video below for the difference. Another oddity shown in the video is that the global find clipboard's contents are ignored/overwritten when I hit Cmd-F with something selected or with the cursor in a word. In any other Mac app, Cmd-F just brings up the find UI, but does not change the current search string.

vsc-vs-textedit-global-find-clipboard

@roblourens
Copy link
Member

@liyanage This is about the search viewlet (cmd+shift+f) not editor find.

@liyanage
Copy link

liyanage commented Dec 23, 2017

Got it.

I just tried that, and it is not working at all there. It never gets the global clipboard contents. This is not in an insiders build however, but in the currently shipping 1.19.0.

@roblourens
Copy link
Member

It's only implemented in the Insiders build.

@wprater
Copy link
Contributor Author

wprater commented Dec 29, 2017

@roblourens Im still seeing broken behavior, should I open a new bug?

If you have the findAll pane open and highlight text and him cmd+e, it will not be populated on all projects find and findAll inputs, but it likely should.

@roblourens
Copy link
Member

It is for me, when the text input box is focused. Should it be set immediately instead?

@wprater
Copy link
Contributor Author

wprater commented Dec 29, 2017

ahh, I see what you're saying now. Its working like that for me as well. however, I think its a better UX to see the input fields update after you set the global buffer vs. on focus.

I think this leads me to another bug, where after I hit cmd+e the find inout field is focused. feels like odd behavior, as I use it to set the global buffer, but I may still wish to navigate around the editor pane a bit.

@roblourens
Copy link
Member

cmd+e is the same as cmd+f, so that's just a missing feature. I don't know whether it has an issue, you can open one (for @rebornix) if you want.

I'm not sure about setting the search viewlet text immediately - it's very visible and if it takes the text from the editor find widget every time I do a search in a file, then people will definitely file bugs about that. Filed #40955 to consider it some more.

@ramya-rao-a ramya-rao-a added the verification-needed Verification of issue is requested label Jan 30, 2018
@bpasero bpasero added the verified Verification succeeded label Feb 1, 2018
@bpasero
Copy link
Member

bpasero commented Feb 1, 2018

I am glad this is turned off by default, needs "search.globalFindClipboard": true

@aeschli
Copy link
Contributor

aeschli commented Feb 1, 2018

But the default settings are not consistent...

  // Controls if the Find Widget should read or modify the shared find clipboard on macOS
  "editor.find.globalFindClipboard": true,

  // Controls if the Search Viewlet should read or modify the shared find clipboard on macOS
  "search.globalFindClipboard": false

@bpasero
Copy link
Member

bpasero commented Feb 1, 2018

Yeah indeed.

@roblourens
Copy link
Member

@rebornix and I thought that it makes more sense in the find widget because of the way it's used. It's only in the search viewlet where it annoys me. And it matches how it's enabled by default in other editors.

@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality search Search widget and operation issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

8 participants