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

User-defined 'matcher' interface #430

ALSchwalm opened this issue Jul 12, 2017 · 1 comment

User-defined 'matcher' interface #430

ALSchwalm opened this issue Jul 12, 2017 · 1 comment


Copy link

ALSchwalm commented Jul 12, 2017

I've written a patch that demonstrates what I'm proposing here, but things are moving a bit too quickly for me to avoid conflicts. I'll issue a PR if this seems like a good idea. Generally, the idea is to implement a user-defined map of functions that are used as the filters for arguments, variables, redirects, and indexes. Currently, there are two 'global' matchers, the default 'prefix matcher' and the 'subsequence matcher' that can be activated by setting -use-subseq-matcher. This is somewhat limiting.

With this patch, you can, for example, allow case-independent completion for variables by setting:

edit:-matcher[variable] = {
  put (re:match '(?i)^'$args[0] $args[1])

So arg 0 is the current name being completed, and arg 1 is the candidate.

The downside is that things are a bit slower. Particularly, I do not provide an option for matching on commands, because there is too much lag (on a system with ~4500 commands).

If this seems like a good strategy, let me know and I'll open a PR.

Copy link

xiaq commented Jul 12, 2017

Hi, thanks for doing this.

I've looked at your current code and it looks good. One thing I am a bit concerned of is that there seems to be a bit too much copying in the conversion and filtering of candidates (not that the bottleneck is the Go part now, but it also introduces quite some boilerplates). Consider combining the cooking and filtering in one step, and depend on a callback rather than []rawCandidate for input:

func cookAndFilterCandidates(n int, func get(int)rawCandidate, matcher eval.Callable, pattern string) {

In terms of the API, you can take advantage of the pipe: pass the candidates as inputs to the matcher and let the matcher do the filtering, instead of calling the matcher every time. You can save some overheads in function calls this way. In particular, in the case of builtin matchers, this should make the function calling overhead O(1) instead of O(n). Admittedly user-defined matchers will likely use each which brings its own overhead, but we could optimize each later.

One last thing: please try to make sure that there is no big performance regression when using builtin matchers.

FYI, there is #220, but it has been dupped against this issue. And BTW we now have some developer discussion groups (documented in the README), you can join one if you like.

Thanks again! I look forward to your PR :)

@xiaq xiaq closed this as completed in 9298098 Jul 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

No branches or pull requests

2 participants