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

counsel-git-grep doesn't wait for three chars on Windows #786

Open
parbo opened this issue Nov 19, 2016 · 8 comments
Open

counsel-git-grep doesn't wait for three chars on Windows #786

parbo opened this issue Nov 19, 2016 · 8 comments

Comments

@parbo
Copy link

parbo commented Nov 19, 2016

Because counsel--git-grep-count is set to 0 when system-type is windows-nt, counsel-git-grep doesn't wait for three chars on big repos on windows. It takes 20s or so to open counsel-git-grep on my +1M line project, and quite a lot of time for each char until the number of matches are reduced.

Maybe it should rather be set to a number > 20000? Or it could be possible to customize the behavior?

@abo-abo
Copy link
Owner

abo-abo commented Nov 19, 2016

It used to wait on Windows, but it wasn't working very well, so I disabled it. If you can make it work well, a PR is welcome.

@parbo
Copy link
Author

parbo commented Nov 19, 2016

I could try. My point is that the current behavior defeats the purpose of not counting the lines, since it will now do a grep of the empty string. Which has the same effect. I think it would be better to assume the repo is large than to assume it's small.

@abo-abo
Copy link
Owner

abo-abo commented Nov 20, 2016

since it will now do a grep of the empty string

That's what I wanted it to do. The standard method of calling a new grep process after each keystroke does not work well on Windows.

@parbo
Copy link
Author

parbo commented Nov 20, 2016

So the grep process is re-used? Can't you spawn that process at three chars instead, so that the initial grep isn't horribly slow?

@abo-abo
Copy link
Owner

abo-abo commented Nov 20, 2016

So the grep process is re-used? Can't you spawn that process at three chars instead, so that the initial grep isn't horribly slow?

It's not reused, it's spawned again. Which is fine on Linux but bad on Windows with its horrible process management. I could spawn the process at 3 chars, but then you can't edit the input out of these 3 chars: they will always be matched.

@fpopineau
Copy link
Contributor

@abo-abo Why do you say it wasn't working well on Windows? I disabled the 0 count for windows-nt and tried a couple of counsel-git-grep in the emacs git repository. It seems to work fine, even with 20000 results.

@abo-abo
Copy link
Owner

abo-abo commented Dec 24, 2016

@fpopineau Maybe I got a glitch. Might need further testing. What Git and Emacs versions you use?

@fpopineau
Copy link
Contributor

Git version is:

$ git --version
git version 2.10.2

and emacs is 25.1 from git repo emacs-25 branch, compiled under msys2/mingw64.

jeberger added a commit to jeberger/swiper that referenced this issue Oct 12, 2017
This is probably a good idea performance-wise since it limits the number
of collection updates when the user is typing. However, the main reason
for this change is to work around an Emacs freeze on Windows when the
dynamic collection is generated by an external command (see for example
this bug in helm-ag which also affects counsel-ag and similar commands:
emacsorphanage/helm-ag#188).

Related to abo-abo#1218 (but only applies for dynamic collections, where the
difference is really noticeable).
Helps with abo-abo#1198 and abo-abo#786.
abo-abo pushed a commit that referenced this issue Nov 13, 2017
This is probably a good idea performance-wise since it limits the number
of collection updates when the user is typing. However, the main reason
for this change is to work around an Emacs freeze on Windows when the
dynamic collection is generated by an external command (see for example
this bug in helm-ag which also affects counsel-ag and similar commands:
emacsorphanage/helm-ag#188).

Related to #1218 (but only applies for dynamic collections, where the
difference is really noticeable).
Helps with #1198 and #786.

Fixes #1237
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

No branches or pull requests

3 participants