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

Search Feature #754

Merged
merged 10 commits into from
Jul 4, 2024
Merged

Search Feature #754

merged 10 commits into from
Jul 4, 2024

Conversation

jeffrey-asm
Copy link
Collaborator

The following implements a trivial search feature into the VZCode codebase. Various icons have been added based on the supported Code Mirror file extensions, and any unsupported file extensions that appear in any search will pivot to a default file icon. The search feature required the entire sidebar UI to be updated as shown below.

After a successful non-empty search, users can toggle the visibility of a given search file containing the line matches to be entirely visible, folded, or removed from the view. When clicking any line for the given matched file, the file becomes active, and the exact pattern is highlighted within the code editor.

There are still plenty of opportunities to improve the current implementation, such as adding visuals to toggle matching exact cases, support for regular expressions, etc.

image

@jeffrey-asm jeffrey-asm linked an issue Jun 28, 2024 that may be closed by this pull request
Copy link
Contributor

@curran curran left a comment

Choose a reason for hiding this comment

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

Absolutely incredible work! This is next level amazing. Thank you so so much!

Some ideas for future improvements:

  • Auto-focus the input field after clicking search
  • Introduce a keyboard shortcut to trigger search
  • Make the search results navigable with the keyboard
  • Use the new icons in the sidebar file listings, not just in search results

// Only conduct a search after fully mounting and entering a new non-empty pattern
setIsSearching(pattern.trim().length >= 1);

// Search about 2 seconds after entering a new pattern
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this could be set to a shorter time, maybe .5 seconds.

gap: 8px;
padding: 8px;

* {
Copy link
Contributor

Choose a reason for hiding this comment

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

This one feels weird.

justify-content: space-between;
align-items: center;

* {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah somehow this pattern of * does not feel good.

@@ -73,6 +79,22 @@ export type VZAction =
| { type: 'set_is_settings_open'; value: boolean }
| { type: 'set_is_doc_open'; value: boolean }

// `set_is_search_open`
// * Sets whether the search tab is open.
| { type: 'set_is_search_open'; value: boolean }
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice approach!

const matches = [];

for (let j = 0; j < lines.length; j++) {
const index = lines[j].indexOf(pattern);;
Copy link
Contributor

Choose a reason for hiding this comment

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

Very nice implementation for search! I'm curious how fast this is, as it's a brute force algorithm. It's probably fine, but I'd be curious to make unit tests for this function and see how fast it is for various sizes of file trees.

src/server/index.js Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search
2 participants