Find in current file command #128915
Find in current file command #128915
Conversation
|
@rebornix, should I rebase to fix the conflict? |
rebornix
added
editor-find
|
I agree that the icon choice here is pretty hard.
Based on that and my writeup at #14836 (comment):
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Thanks for taking the time to review this @meche-gh. However, I'm not going to make any changes to this PR at this time. I still would like it merged at some point, but that will need to be done by someone from Microsoft and I haven't heard any feedback since I opened this about 1.5 years ago. If I hear back from someone there and it seems like they are willing to merge it, then I'll take the time to make updates. |
|
Makes sense. This pull's assigned and I'm the review has pinged them already :) will give it a wait |
There was a problem hiding this comment.
I'm wrong about the scope of this PR & I unfortunately read the last few messages of #14836 first.
I see that this pull is specifically for functionality #1 - the "find" op.
Gonna see if I can dismiss my review and replace it with just the icon change.
| @@ -47,6 +47,7 @@ export const findReplaceIcon = registerIcon('find-replace', Codicon.replace, nls | |||
| export const findReplaceAllIcon = registerIcon('find-replace-all', Codicon.replaceAll, nls.localize('findReplaceAllIcon', 'Icon for \'Replace All\' in the editor find widget.')); | |||
| export const findPreviousMatchIcon = registerIcon('find-previous-match', Codicon.arrowUp, nls.localize('findPreviousMatchIcon', 'Icon for \'Find Previous\' in the editor find widget.')); | |||
| export const findNextMatchIcon = registerIcon('find-next-match', Codicon.arrowDown, nls.localize('findNextMatchIcon', 'Icon for \'Find Next\' in the editor find widget.')); | |||
| export const findAllMatchesIcon = registerIcon('find-all-in-current-file', Codicon.search, nls.localize('findAllMatchesIcon', 'Icon for \'Find All in Current File\' in the editor find widget.')); | |||
There was a problem hiding this comment.
| export const findAllMatchesIcon = registerIcon('find-all-in-current-file', Codicon.search, nls.localize('findAllMatchesIcon', 'Icon for \'Find All in Current File\' in the editor find widget.')); | |
| export const findAllMatchesIcon = registerIcon('find-all-in-current-file', Codicon.newFile, nls.localize('findAllMatchesIcon', 'Icon for \'Find All in Current File\' in the editor find widget.')); |
There was a problem hiding this comment.
@ssigwart I understand work is on pause for this PR but could you merge this suggestion? It's the correct icon, matching the search-new-editor button in the search pane next to the vertical Activity Bar.
There was a problem hiding this comment.
@zm-cttae, I had taken a look at this when you first suggested, but this branch is so far behind main now and there were major merge conflicts. It's not worth my time fixing that without some type of feedback from Microsoft indicating this PR may be merged.
There was a problem hiding this comment.
It'd be possible to make the icon change and resolve the review w/o merge.. looking to keep my lists clear
EDIT: Unsubscribe also works
There was a problem hiding this comment.
It doesn't seem that useful to change the icon when the whole thing won't work in its current state due to VS Code internal changes since I originally wrote this. I assume that Microsoft has other priorities and won't be looking at this, but I leave it open just in case.
We use `expand-all` for the "select matches" icon. We also rename from "find all" to "select all" for microsoft#128915. Refs: ec02bcd & microsoft#14836
|
Is this still relevant? |
|
I still think it's a viable solution to #14836, which is still open. However, it would need merge conflicts fixed first. I don't want to spend time doing that until someone from Microsoft indicates that this PR had some likelihood of being merged. |
This PR fixes #14836.
I did not read all of the comments on that issue since it's extensive, but I implemented @alexdima's suggestion in #14836 (comment).
This adds a way to find occurrences in the current file. To test, you can open the find widget, then click the new icon to find in the file. I wasn't sure what icon to use. The two I thought fit best were
fileorsearch.I went with
search, but I'm open to suggestions.