Minimum length of input?
Closed this issue · 16 comments
If I type fewer than three characters, I get no matches. I can't with a quick look at the source find any setting or even code that controls this; is it rather a helm thing? I'd like to be able to get matches for shorter search strings! I'm using git grep in a git project, and git grep itself seems to have no problem finding shorter matches.
Thanks very much for this package; searching with helm like this is magic!
Found it! It's the :requires-pattern property of helm-build-async-source. Would you accept a patch to allow this number to be configured? Or is there any reason it can't be 2 or even 1?
Thanks for your interest in this package!
It seems I took the hard-coding of that value from some existing helm packages. With a small search from in my elpa-directory, quite a few have the value of 2 for that :requires-pattern. I have no recollection why 3 was used for this.
There is no reason it couldn't be less than 3, and I can accept a change for a defcustom for that (with the unchanged default value).
I can close this issue now, and thanks for the new release!
My fix is not correct. The customized value does not affect the default, because helm-grepint-helm-source is evaluated at compile time, with the default value of helm-grepint-min-pattern-length. I'm not sure what the best way to fix this is.
Hmm.. it is indeed so that setqing helm-grepint-min-pattern-length before requiring helm-grepint does not change the value inside helm-grepint-helm-source.
However, there is also helm-grepint-candidate-number-limit which is an equivalent variable and is also given to helm-grepint-helm-source and that works. This needs more investigation.
Oops.. I was typoing the variable name in my testing setup 😅. Setting the variable seems to work. E.g.:
(setq helm-grepint-min-pattern-length 1)
(setq helm-grepint-candidate-number-limit 2)
(require 'helm-grepint)
(helm-grepint-set-default-config-latest)
(global-set-key (kbd "C-c g") #'helm-grepint-grep)
With this, both variables seem to be properly set when running helm-grepint-grep.
Note that if you are using use-package, then IIUC the variables need to be set in the :init -phase (which is run before requiring the package).
Thanks for the workaround. But is it possible to make this work as a normal defcustom?
To my understanding this is the normal way defcustoms are used.
I.e. they are variables that configure the behaviour of a package and the user's init.el sets a value to that variable before loading the package.
Or do you mean using customize to set the defcustoms ?
That's exactly it, it should be possible to use customize.
Most likely the problem is that customize pastes the setting of the custom variables to the end of the init.el. I.e. after everything else.
Having a separate custom file has the advantage that it can be loaded earlier:
https://www.gnu.org/software/emacs/manual/html_node/emacs/Saving-Customizations.html
E.g. with the following steps:
- init.el with the following kind of changes:
(setq custom-file (concat user-emacs-directory "custom.el"))
(when (file-exists-p custom-file)
(load custom-file))
(require 'helm-grepint)
(helm-grepint-set-default-config-latest)
(global-set-key (kbd "C-c g") #'helm-grepint-grep)
- Run
customizeand set the variables. It should fill thecustom.el. - Restart emacs.
Now helm-grepint should be configured properly.
To get it working without restarting emacs the helm-grepint-grep-source should be refactored in such a manner that the async source should be built (with helm-build-async-source) per-call of the functionality. I don't want to do such a refactoring, for the following reasons:
- The
defvaring of the contents ofhelm-build-async-sourceis a recurring pattern in helm. E.g. inhelm-x-files.elandhelm-find.el. - The problem can be worked around with the above ways.
- The existing
helm-grepint-helm-sourceis part of the packages public interface and I would not want to delete it or modify it significantly, unless there is no other way (and preferably that would mean a major version bump).
I already had a separate custom.el, but previously I loaded helm-grepint before it, because I do my "manual" customizations before loading custom.el.
So I reversed this, and it works!
The only other confusing thing is the requirement to use an init function. It would be fine, I think, if the defaults changed from one version to another of the package, and anyone who cares can configure them. (I have not come across many packages that do this "opt-in defaults" thing, I admit.) At least, it seems to conflict with customization, as it's a non-optional step: you can't simply customize the package and then expect it to load automatically, as most packages will.
I don't really use customize to configure packages, and that (unfortunately) shows in the design.
I use the use-package (https://github.com/jwiegley/use-package) mechanism, and that that has different phases where custom code is run. I.e. Run :init code before the package is run (setq variables), load the package, run the :config code after package run (run the init functions and set the keybindings).
The reason for the init functions (i.e. helm-grepint-set-default-config and helm-grepint-set-default-config-latest) are:
- The package doesn't force any grep configuration to the user. E.g. some might use mercurial and ripgrep making the default configurations of git-grep and ag unnecessary.
- It is more backwards compatible as there can be configurations of different versions present (e.g.
helm-grepint-set-default-config-v1.0.0). - Those functions serve as an example how to configure the package to a large extent. E.g. for adding support to ripgrep or pt.
I understand that as a developer you're in a bind: either you freeze the default configuration, or you annoy users when you change defaults which they have not configured (so they are exposed to the change). Nevertheless, this is how Emacs itself works, so I think the right thing for most packages to do is to follow that. Similarly with the customize interface: that's the default, so packages should work with it. Usually the way Emacs itself tries to avoid annoying users is by changing defaults only rarely, and normally just offering new options (as you did for me on this occasion!).
One thing I found in particular confusing about helm-intgrep is that if I don't configure it at all, it doesn't work! (It complains it can't find a suitable grep program.) I don't see any harm in having a default configuration that defaults to git grep in git directories and grep otherwise; apart from anything else, that gives a putative Mercurial and grep user something to compare against if, when they change the configuration, it doesn't work. And to see how to configure the package, I'd normally rely on M-x customize <name of package>! It shouldn't be necessary to write (or read) elisp to configure an Emacs package.
I opted in having no configuration by default (while also providing (helm-grepint-set-default-config) to actually configure the package), because the original motivation for writing the package was to combine the features of helm-git-grep and helm-ag. Both of those are locked in to the mentioned programs and I wanted to avoid that.
The package commentary first describes what the package does and next it documents how to enable the described features (the four lines of elisp).
In the best cases it might not be needed to interface with elisp to work with a package. I.e. with customize one can change variables, fonts and themes (https://www.gnu.org/software/emacs/manual/html_node/emacs/Easy-Customization.html#Easy-Customization), but, for example, I don't see a way to define key bindings with that. Therefore if any advanced customization is needed, elisp is usually needed to configure.
I agree that elisp will be needed to change key bindings typically (this is unfortunate!). And of course your motivation to avoid lock-in to a particular grep is also excellent. Thanks very much for taking the time to explain your design choices in such depth!
(I think this can be closed again!)