Skip to content

add fill selection text for quick search#191956

Merged
andreamah merged 14 commits into
microsoft:mainfrom
Kalmaegi:quick_search
Sep 14, 2023
Merged

add fill selection text for quick search#191956
andreamah merged 14 commits into
microsoft:mainfrom
Kalmaegi:quick_search

Conversation

@Kalmaegi

@Kalmaegi Kalmaegi commented Sep 1, 2023

Copy link
Copy Markdown
Contributor

For: #191513

@Kalmaegi

Kalmaegi commented Sep 1, 2023

Copy link
Copy Markdown
Contributor Author

This is a simple version, there are still many problems to solve, i.e. if we select a word in the editor, but we switch the cursor to the left search window, and let the cursor in the input box, the shortcut search will still fill the editor selected word, I think it should not be filled at this time, so I will add judgment for editor is focus

@andreamah andreamah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks again for working on this! Just some small comments.

Comment thread src/vs/workbench/contrib/search/browser/searchActionsTextQuickAccess.ts Outdated
Comment thread src/vs/workbench/contrib/search/browser/searchActionsTextQuickAccess.ts Outdated
@Kalmaegi
Kalmaegi requested a review from andreamah September 6, 2023 05:48

@andreamah andreamah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! just a few more nitpicks

Comment thread src/vs/editor/contrib/find/browser/findController.ts Outdated
@Kalmaegi

Kalmaegi commented Sep 8, 2023

Copy link
Copy Markdown
Contributor Author

Thanks! just a few more nitpicks

These are all help to correct my code progress ~

@andreamah andreamah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for not commenting on this earlier - I just noticed this while testing your PR.

@andreamah

Copy link
Copy Markdown
Contributor

@TylerLeonhardt This PR looks fine to me, but I did notice that this doesn't work if preserveInput is true. This is because preserveInput takes precedence over any default string (even if defaultFilterValue is defined).

if (defaultFilterValue === DefaultQuickAccessFilterValue.LAST) {
newValue = this.lastAcceptedPickerValues.get(descriptor);
} else if (typeof defaultFilterValue === 'string') {
newValue = `${descriptor.prefix}${defaultFilterValue}`;
}

Should we edit this somehow, or is this intentional?

Comment thread src/vs/workbench/contrib/search/browser/searchActionsTextQuickAccess.ts Outdated
Comment thread src/vs/workbench/contrib/search/browser/searchView.ts Outdated
@TylerLeonhardt

Copy link
Copy Markdown
Member

@andreamah that seems backwards to me. Can you get an issue going and I'll take a look deeper?

@TylerLeonhardt

Copy link
Copy Markdown
Member

Excited to see this land! Thanks, @weartist!

@vscodenpa vscodenpa added this to the September 2023 milestone Sep 12, 2023
@andreamah
andreamah merged commit 23605b1 into microsoft:main Sep 14, 2023
lins0621 pushed a commit to lins0621/vscode that referenced this pull request Sep 14, 2023
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants