Skip to content

Add Indexing Progress, Index Off Main Thread #1501

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

Merged
merged 6 commits into from
Dec 11, 2023

Conversation

thecoolwinter
Copy link
Collaborator

@thecoolwinter thecoolwinter commented Dec 5, 2023

Description

  • Explicitly do all indexing work off the main thread, including gathering file URLs.
  • Iteratively load files into memory for indexing, rather than loading all files in the workspace at once.
  • Add a published indexing progress variable.
  • Add a progress bar for indexing that takes enough time. For most small projects this will never be shown, for large projects it's a helpful indicator.

Related Issues

  • No issues.

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

option.1.final.mov

LeoBorai
LeoBorai previously approved these changes Dec 6, 2023
Copy link
Member

@LeoBorai LeoBorai left a comment

Choose a reason for hiding this comment

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

LGTM!

@tom-ludwig
Copy link
Member

@thecoolwinter Nice progress bar, I really like it! I saw Xcode shows indexing progress at the top tho. Ever thought about using that UI?
Screenshot 2023-12-06 at 3 39 29 PM

@thecoolwinter
Copy link
Collaborator Author

@activcoding yes! I would like to use a progress bar like that but we don't have the activity bar or any of the necessary API implemented. When that is finished this progress bar should be removed.

Copy link
Member

@tom-ludwig tom-ludwig left a comment

Choose a reason for hiding this comment

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

Looks good so far. I'm unsure about resetting the results at the start of a search tho; it seems a bit odd when a search takes longer than 0.2 seconds. Is there a specific reason not to replace the results once they are found?

@thecoolwinter
Copy link
Collaborator Author

Looks good so far. I'm unsure about resetting the results at the start of a search tho; it seems a bit odd when a search takes longer than 0.2 seconds. Is there a specific reason not to replace the results once they are found?

In massive workspaces I was finding search could take longer if the search query was small (like searching for //) and there were enough matches that finding each match in each file took long enough. In most searches it isn't necessary I agree.

Copy link
Member

@tom-ludwig tom-ludwig left a comment

Choose a reason for hiding this comment

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

Looks good now, all set for merging.

@austincondiff austincondiff merged commit f38bc67 into CodeEditApp:main Dec 11, 2023
@thecoolwinter thecoolwinter added the bug Something isn't working label Dec 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants