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

Support quick file-search with go to line #9478

Merged
merged 1 commit into from
May 19, 2021

Conversation

alvsan09
Copy link
Contributor

@alvsan09 alvsan09 commented May 13, 2021

What it does

The quick file-search text box (normally accessed via Ctrl + P)
now allows the option to append filter input separators to indicate
the target line number and column. This follows the current vscode
patterns.

E.g.
:<line number?>:<column?>
#<line number?>#<column?>
:<line number?>,<column?>

Fixes: #9476.

How to test

The 'gif' attached shows the new facility available in the quick file-search box, verify that:

  • The input box now indicates the option to 'append : to go to line'.
  • type a file path pattern, append ':' and a line number. Check that the selected file is opened at the given line number.
  • Adding alphabetical characters instead of numerical for the go to line number are ignored.
  • type a file path pattern, append ':' for the line number and a second separator ':' for the column. Check that the selected file is opened at the given line number and column.
  • Use '#' character as separators and repeat the check.
  • the ',' comma character can be used as well as an alternative separator for column.
  • Search with no file filter simply line and column (e.g. :15:4) and verify that the currently selected file opens at the given location.
  • Verify that entering a file filter with no line number navigates to the top of the file (regression test).
  • Make sure that file filter searches with spaces continue to work with and without given line and column.

goto_line

Review checklist

Reminder for reviewers

Signed-off-by: Alvaro Sanchez-Leon alvaro.sanchez-leon@ericsson.com

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves file search issues related to the file search labels May 13, 2021
@alvsan09 alvsan09 force-pushed the file-search-go-to-line branch 3 times, most recently from d133606 to 3530670 Compare May 14, 2021 23:11
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I confirmed that the changes work well functionality 👍
I have some minor comments about variable names, let me know if you disagree.

packages/file-search/src/browser/quick-file-open.ts Outdated Show resolved Hide resolved
packages/file-search/src/browser/quick-file-open.ts Outdated Show resolved Hide resolved
@alvsan09 alvsan09 force-pushed the file-search-go-to-line branch 2 times, most recently from fca2dcc to e642139 Compare May 17, 2021 20:20
The quick file-search text box (normally accessed via Ctrl + P)
now allows the option to append filter input separators to indicate
the target line number and column. This follows the current vscode
patterns.

E.g.
<file filter>:<line number?>:<column?>
<file filter>#<line number?>#<column?>
<file filter>:<line number?>,<column?>

Signed-off-by: Alvaro Sanchez-Leon <alvaro.sanchez-leon@ericsson.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The changes work correctly for me, and I verified the code as well 👍

  • verified pattern:line format.
  • verified pattern:line:column format.
  • verified pattern:line,column format.

Offline we spoke about a potential future improvement where we open the editor with a buffer (display previous content) instead of the selected line being the first in the document. Would you mind opening an issue to track this feature once the pull-request is merged?

@alvsan09
Copy link
Contributor Author

Offline we spoke about a potential future improvement where we open the editor with a buffer (display previous content) instead of the selected line being the first in the document. Would you mind opening an issue to track this feature once the pull-request is merged?

The issue #9495 is now created to follow up and address this pending issue.

@alvsan09
Copy link
Contributor Author

@vince-fugnitto, @marechal-p,
is there anything pending in this PR before moving forward?

@paul-marechal paul-marechal merged commit 960a521 into eclipse-theia:master May 19, 2021
@vince-fugnitto vince-fugnitto added this to the 1.14.0 milestone May 27, 2021
Copy link

@emilyann211 emilyann211 left a comment

Choose a reason for hiding this comment

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

Example PKGBUILD

@vince-fugnitto
Copy link
Member

@emilyann211 did you comment by accident?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves file search issues related to the file search
Projects
None yet
Development

Successfully merging this pull request may close these issues.

file-search: support 'go to line' directly when searching for files
4 participants