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 with selected values when reveal TerminalFind #190466

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

tisilent
Copy link
Contributor

fix #189343

reveal TerminalFind
meganrogge
meganrogge previously approved these changes Aug 15, 2023
@@ -81,9 +81,9 @@ export class TerminalFindWidget extends SimpleFindWidget {
override reveal(): void {
const initialInput = this._instance.hasSelection() && !this._instance.selection!.includes('\n') ? this._instance.selection : undefined;
const xterm = this._instance.xterm;
if (xterm && this.inputValue && this.inputValue !== '') {
if (xterm && initialInput && initialInput !== '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to fallback to this.inputValue if initialInput is undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need to adjust and test it.

connor4312
connor4312 previously approved these changes Aug 15, 2023
@meganrogge meganrogge requested a review from Tyriar August 15, 2023 17:14
 initialInput is undefined
@tisilent tisilent dismissed stale reviews from connor4312 and meganrogge via 004189e August 16, 2023 09:16
@tisilent
Copy link
Contributor Author

hi @meganrogge , fallback to this.inputValue, It can improve the following operating experience.

  1. reveal
  2. hide
  3. input command
  4. reveal

@Tyriar Tyriar added this to the August 2023 milestone Aug 16, 2023
Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

Works great, thanks again @tisilent 👏

@Tyriar Tyriar enabled auto-merge August 16, 2023 14:55
@Tyriar Tyriar merged commit d4e89fd into microsoft:main Aug 16, 2023
5 of 6 checks passed
@tisilent tisilent deleted the fix-terminalFinad-initial-search branch August 16, 2023 15:59
@github-actions github-actions bot locked and limited conversation to collaborators Sep 30, 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.

Integrated console find feature uses the previous search
5 participants