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

Bump file picker limit #171

Merged
merged 1 commit into from
Jun 8, 2021

Conversation

hovsater
Copy link
Contributor

@hovsater hovsater commented Jun 7, 2021

As a quick fix before we decide what to do next in #52, I've bumped the limit.

@archseer archseer merged commit 08f5031 into helix-editor:master Jun 8, 2021
@hovsater hovsater deleted the fix/bump-file-picker-limit branch June 8, 2021 07:40
@pickfire
Copy link
Contributor

pickfire commented Jun 8, 2021

What if we just remove the limit?

@archseer
Copy link
Member

archseer commented Jun 8, 2021

It would be really bad if you triggered space+f in / then, since it would load your entire filesystem metadata into memory. I think having an upper bound, even if very large one is good. This bound is also applied after processing .gitignored files (build/, target/, etc. etc.) so hopefully there shouldn't be millions of files.

@pickfire
Copy link
Contributor

pickfire commented Jun 8, 2021

It would be really bad if you triggered space+f in / then, since it would load your entire filesystem metadata into memory. I think having an upper bound, even if very large one is good. This bound is also applied after processing .gitignored files (build/, target/, etc. etc.) so hopefully there shouldn't be millions of files.

Okay, what about this. We only apply a the upper bound if .git isn't found. Even though the project have like millions of file it's still under the project and excluding those git-ignored. At least in this case it's the project file. Maybe we should try running it (both debug and release) without upper bound in linux and rust repository and see if it's good.

@hovsater
Copy link
Contributor Author

hovsater commented Jun 8, 2021

@pickfire I find that a compelling alternative. Especially since I believe the ignore crate reads .ignore files in addition to the .gitignore.

@pickfire
Copy link
Contributor

pickfire commented Jun 8, 2021

I never heard about .ignore but I think .gitignore should be good enough but we should also have a way to show those ignored files.

@hovsater
Copy link
Contributor Author

hovsater commented Jun 8, 2021

@pickfire .ignore files have the same semantics as gitignore files and are supported by search tools such as ripgrep and The Silver Searcher. Allowing searching ignored files would be possible using the WalkBuilder.

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.

None yet

3 participants