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

First draft: consult-hippie #175

Closed
wants to merge 1 commit into from
Closed

First draft: consult-hippie #175

wants to merge 1 commit into from

Conversation

minad
Copy link
Owner

@minad minad commented Jan 22, 2021

@protesilaos This is a first draft for a consult-hippie command taken from here https://gist.github.com/JohnLunzer/7c6d72a14c76c0a3057535e4f6148ef8. I believe it originated in the Emacs wiki (TODO: Check license). I am not fully convinced by this command, since it does not seem like a significant improvement over dabbrev-completion. But I am interested in feedback!

@minad minad mentioned this pull request Jan 22, 2021
20 tasks
@protesilaos
Copy link
Sponsor

I am not fully convinced by this command, since it does not seem like a significant improvement over dabbrev-completion. But I am interested in feedback!

Thanks! I will pull in the changes and start testing.

@protesilaos
Copy link
Sponsor

Very quick first comparison between dabbrev-completion and consult-hippie:

Dan<dabbrev-completion>

Daniel's
Daniel

And:

Dan<consult-hippie>

Daniel Mendler's Consult is a welcome addition to the ecosystem of
Daniel
Dance
Dancing
Dangerous
Daniel's

I will need more time testing this with some real world situations. The only observation right now is whether a list could be broken into sublists. So this:

Daniel Mendler's Consult is a welcome addition to the ecosystem of

Becomes something like:

Daniel Mendler's Consult is a welcome addition to the ecosystem of
Daniel Mendler's Consult
Daniel Mendler's

Current hippie config:

(setq hippie-expand-try-functions-list
        '(try-expand-list
          try-expand-line
          try-expand-dabbrev-all-buffers
          try-expand-dabbrev
          try-expand-dabbrev-from-kill
          try-expand-all-abbrevs
          try-complete-lisp-symbol-partially
          try-complete-lisp-symbol
          try-complete-file-name-partially
          try-complete-file-name))

@minad
Copy link
Owner Author

minad commented Jan 22, 2021

Sure, I could manually split the candidates in consult-hippie. However it would be more principled if you write a special try-* function which produces such candidates.

About the examples you showed, I am still unsure about the use case case. For example if you type "Dan" and want to complete to "Dance", this is not helpful at all, since you need three key presses of <down> to arrive at the desired candidate. It is more useful with longer candidates obviously. For example when completing function names or long words, e.g., after typing "consult--". Maybe very short candidates or candidates which are only few characters longer than the original input should not even be shown.

@protesilaos
Copy link
Sponsor

About the examples you showed, I am still unsure about the use case case. For example if you type "Dan" and want to complete to "Dance", this is not helpful at all, since you need three key presses of to arrive at the desired candidate. It is more useful with longer candidates obviously.

You are right! Those are not offering an obvious benefit.---and is why I would need more time to test this in real scenaria. Back when I was using hippie-expand more regularly, what I liked the most was completing longer stretches of text, like:

`(con<consult-hippie>

(consult-view ((,class :inherit bold :foreground ,fg-special-warm)))
(consult-preview-line ((,class :inherit modus-theme-special-mild)))
(consult-preview-error ((,class :inherit modus-theme-intense-red)))
(consult-preview-cursor ((,class :inherit modus-theme-intense-blue)))
...

The reason I stopped using it was because it did not offer completion, so it was more miss than hit.

By the way, there is a bug currently. If the text starts with a parenthesis, then that gets added again. For instance:

`(con<consult-hippie>
`((consult-view ((,class :inherit bold :foreground ,fg-special-warm)))

@minad
Copy link
Owner Author

minad commented Jan 22, 2021

Back when I was using hippie-expand more regularly, what I liked the most was completing longer stretches of text.

Yes, this is exactly where I see the largest benefit of dabbrev and hippie.

By the way, there is a bug currently. If the text starts with a parenthesis, then that gets added again. For instance:

Okay, this whole thing certainly needs a good cross check. Basically I took the verbatim version from the gist above with some minor improvements to make it faster (inhibit modification etc). Could it be that the parentheses are introduced by paredit/smartparens or some other similar package?

@protesilaos
Copy link
Sponsor

Yes, this is exactly where I see the largest benefit of dabbrev and hippie.

Let's see what the tests reveal. By the way, have you notified the people on the wishlist thread? Maybe someone else would like to give it a try.

Could it be that the parentheses are introduced by paredit/smartparens or some other similar package?

I don't use those or anything similar that auto-inserts characters.

@minad
Copy link
Owner Author

minad commented Jan 22, 2021

Let's see what the tests reveal. By the way, have you notified the people on the wishlist thread? Maybe someone else would like to give it a try.

Let's also summon @clemera, @hmelman and @oantolin here, since they took part in the hippie/dabbrev discussion in #6.

@hmelman
Copy link

hmelman commented Jan 22, 2021

I'll play with this. FWIW, when I used hippie-expand I tended to use it for symbol names, not for multi-word strings. Even in this case it's useful to automagically get the case correct or to deal with more difficult to type punctuation. Also I believe hippie enabled the use of text in other (visible) windows/buffers before dabbrev got that capability. So it was very convenient to look up the definition or documentation of something in another emacs window and then use hippie to complete that in the current buffer. This was fantastic before there was smart completion-at-point or LSP enabled "IntelliSense" in non-elisp buffers.

Now I use it in markdown-mode and have switched to dabbrev because it completes words better when some instances have markup at their beginning. I haven't been able to make hippie work as well in that case and it's this that led me to find out that the hippie functions have their own dabbrev implementation and don't call through to dabbrev.el.

@manuel-uberti
Copy link

Just for reference, Helm offers helm-dabbrev.el which covered all my use cases for completion most of the times. For the few times it didn't get it right, Company helped.

@minad
Copy link
Owner Author

minad commented Jan 22, 2021

@manuel-uberti What makes this helm-dabbrev special? What does it do differently than dabbrev-completion for example? It seems to bring its complete own dabbrev implementation. Something like this is mostly out of the question for Consult. I would want to reuse either dabbrev or hippie.

@manuel-uberti
Copy link

As I said: just for reference. :)

dabbrev suits me well enough these days.

@protesilaos
Copy link
Sponsor

hmelman: Now I use it in markdown-mode and have switched to dabbrev because it completes words better when some instances have markup at their beginning. I haven't been able to make hippie work as well in that case and it's this that led me to find out that the hippie functions have their own dabbrev implementation and don't call through to dabbrev.el.

manuel-uberti: dabbrev suits me well enough these days.

I should add that I also am a full time dabbrev user (no company-mode or any pop-ups). What I would want from a dabbrev-completion-like command is the ability to complete longer stretches of text more efficiently. For example, to complete a common pattern in my buffer, say, "Protesilaos Stavrou", with dabbrev-expand I must do this:

Pro <dabbrev-expand> SPC <dabbrev-expand>

My vision for consult-hippie (or similar) was that it would basically eliminate the SPC and treat common occurrences of words/symbols as a "phrase" construct. Basically a greedy variant of dabbrev. Though I understand this may well be outside the scope of Consult, in which case consult-hippie may not be of much value over dabbrev-completion.

@minad minad force-pushed the main branch 3 times, most recently from b58638f to 002b267 Compare January 31, 2021 10:11
@minad
Copy link
Owner Author

minad commented Feb 1, 2021

Comment by @protesilaos in https://gitlab.com/protesilaos/modus-themes/-/issues/155

I guess we should move the consult-hippie discussion there when you feel like it. Perhaps hippie-expand is not the right tool for the job and maybe we need a new (or existing?) tool that provides a superset for dabbrev or something like dabbrev but with wider reach. While dabbrev matches words/symbols, this one should be able to match longer patterns on lines. So when the user types "Eur" it shows candidates like:
European Council
European Central Bank
European Coal and Steel Community
European Border and Coast Guard Agency
European body for the enhancement of judicial co-operation
...
Performance penalties could be mitigated by minimum characters and those kind of things you already know.

@minad
Copy link
Owner Author

minad commented Feb 1, 2021

I also asked myself if it would be appropriate to create a new tool, hippie done right. I have at least these criticisms regarding hippie:

  • Hippie does not reuse dabbrev, I assumed it does but it reinvents its own version.
  • Hippie mutates the buffer directly and does not offer an accessible API for completion.
  • Since hippie seems to allow arbitrary expansions, it is too uncontrolled and not reusable.

The idea would be that the package should offer pure functions returning lists of candidates such that it is a better fit for completion. I think this would also require less code since you would only have a single point where the expansion is applied in the target buffer. If someone is looking for a new small Emacs project, this could be it - if it does not already exist :)
The expansions you propose seem useful, but ideally I would like something which is configurable like hippie, but more flexible such that arbitrary backends can be plugged and only useful expansions are generated. Maybe this is something which should be implemented as an autocomplete/company backend?

@oantolin
Copy link
Contributor

oantolin commented Feb 1, 2021

It does sound like a company backend would be appropriate. Is there a way to not have company complete automatically, but only when I run a command? I love completion, but don't particularly enjoy autocompletion.

@minad
Copy link
Owner Author

minad commented Feb 1, 2021

In my configuration company shows this popup and then I have to complete explicitly? Seems to be pretty much the default configuration.

@minad
Copy link
Owner Author

minad commented Feb 1, 2021

What I dislike about company is that only one backend is active at a time. I don't understand why this has to be the case.

EDIT: But given such a hypothetical configurable super-hippo, the hippo could be configured to query multiple backends by itself, so company could be configured to only use hippo.

@protesilaos
Copy link
Sponsor

I think those are fair criticisms and a company backend would be interesting to see. I also remember company would perform manual operations by default, though it could be configured to auto-complete when only a single candidate was available.

Can company somehow work from the minibuffer instead of its typical overaly/child-frame?

@protesilaos
Copy link
Sponsor

hippo is a neat name, by the way.

@oantolin
Copy link
Contributor

oantolin commented Feb 1, 2021

In my configuration company shows this popup and then I have to complete explicitly? Seems to be pretty much the default configuration.

Yes, but the popup pops up automatically as you type. I want it to only appear when summoned. Do you know if I can configure it to do that?

@oantolin
Copy link
Contributor

oantolin commented Feb 1, 2021

Forget company! Can this hippo be implemented as a completion at point function, so it participates in the normal C-M-i?

@oantolin
Copy link
Contributor

oantolin commented Feb 1, 2021

I think I made a mistake: hippo is your name for the multiple backend thing. I claim that is just completion-at-point and completion-at-point-functions is the list of backends.

And what I should have said instead of hippo, is that @protesilaos's "complete longer streches of text" could probably be provided as a function to add to completion-at-point-functions.

@minad
Copy link
Owner Author

minad commented Feb 1, 2021

Yes, but the popup pops up automatically as you type. I want it to only appear when summoned. Do you know if I can configure it to do that?

No idea if there is an option. I guess there is. I want it to popup automatically. For me there a no big downsides. It does not introduce window bumpiness like the minibuffer.

Forget company! Can this hippo be implemented as a completion at point function, so it participates in the normal C-M-i?

Sure that would be good too. Actually the hippo should allow it to be used for both company and completion at point. This is something I dislike about dabbrev-expand/hippie-expand, that they need their own key. These things should be unified.

@oantolin
Copy link
Contributor

oantolin commented Feb 1, 2021

I want it to popup automatically. For me there a no big downsides. It does not introduce window bumpiness like the minibuffer.

True but it does introduce a different kind of bumpiness that's even worse: it happens right at the spot I'm looking at!

@clemera
Copy link
Contributor

clemera commented Feb 1, 2021

I have (setq company-idle-delay nil) in my config, this will stop the auto popup

@dgutov
Copy link

dgutov commented Apr 16, 2021

Company-CAPF probably does not pass the returned candidates through the completion styles at all and simply shows everything directly.

Perhaps I misunderstood you, but company-capf goes through completion-all-completions, so it uses the completion-styles mechanism. Which also makes fuzzy-matching-on-the-server difficult.

@minad
Copy link
Owner Author

minad commented Apr 16, 2021

@dgutov I speculated and it was wrong. Company goes through completion-all-completions which is the right thing to do.

However it seems one is often required to use completion styles which pass a prefix to the backend, e.g., as you say when filtering on the server. In this case one is required to use the basic completion style.

I was wondering if one could support Orderless completion with Company or Corfu by compiling the first word to a prefix and then filter only using the remaining words inside the completion style on the side of Emacs. I made an issue to keep this in mind (minad/corfu#5). cc @oantolin

EDIT: One would probably want a possibility to disable the prefix filtering (special syntax) or only enable it conditionally for completion in region? Alternatively use different styles for completion-in-region and minibuffer completion.

@dgutov
Copy link

dgutov commented Apr 16, 2021

completion-category-defaults and completion-category-overrides (user-defined) decide which completion styles are used, based on the category of the completion table.

Whether completion is "in region" or not, doesn't enter the equation, but I suppose you could override one of these variables with a let-binding in your completion-in-region-function.

@minad
Copy link
Owner Author

minad commented Apr 16, 2021

Whether completion is "in region" or not, doesn't enter the equation, but I suppose you could override one of these variables with a let-binding in your completion-in-region-function.

Yes, this is what I was suggesting.

@astoff
Copy link
Contributor

astoff commented Apr 16, 2021

However it seems one is often required to use completion styles which pass a prefix to the backend, e.g., as you say when filtering on the server.

I'm not sure I understand what you meant here, but let me clarify a bit how completion requests work in LSP. The editor only passes the file, line and column to the server. It's not up to the editor to decide what is constitutes the region being completed. Then the server simply returns a list of candidates.

The fuzzy completions, on the other hand, do not [work with Corfu]...

I take that back, actually! When I hit C-M-i after \cite{partofthetitle I do see the relevant suggestions. If I keep typing, then Emacs tries to refine the list and that immediately results in 0 candidates, since no BibTeX label starts with partofthetitle. But this is already much better than nothing.

@minad
Copy link
Owner Author

minad commented Apr 16, 2021

I take that back, actually! When I hit C-M-i after \cite{partofthetitle I do see the relevant suggestions. If I keep typing, then Emacs tries to refine the list and that immediately results in 0 candidates, since no BibTeX label starts with partofthetitle. But this is already much better than nothing.

I still don't understand fully what you describe now. When you enter \cite{partofthetitle the server returns the relevant completions, which do not match the prefix. But then you shouldn't see the candidates since they already don't match?

@astoff
Copy link
Contributor

astoff commented Apr 17, 2021

Yes, there's indeed something funny going on. From just observing the UI and without further debugging, it seems that I see all candidates returned by the backend only as long as the completion style doesn't match any of the candidates. So:

  • If the completion prefix is foo and the candidates are afoo, fooa and bar (the latter being a fuzzy match, e.g. it's a bibitem whose authors contain bar), then the UI shows only afoo and fooa. It would probably remove fooa as well if I wasn't using orderless.
  • If the completion prefix is foo and the candidates are x, y and z, then:
    • The *Completions* buffer UI claims there are no matches
    • Corfu shows all the 3 candidates.

@minad
Copy link
Owner Author

minad commented Apr 17, 2021

Okay, maybe there is something wrong with which string is used for the filtering. Does this also happen with Company? Both Corfu and Company use completion-all-completions to retrieve the completions.

Corfu shows all the 3 candidates.

In particular this is should not happen.

If the completion prefix is foo and the candidates are afoo, fooa and bar (the latter being a fuzzy match, e.g. it's a bibitem whose authors contain bar), then the UI shows only afoo and fooa. It would probably remove fooa as well if I wasn't using orderless.

So this is a big problem with completion styles. I wonder how Company handles this problem, it should be affected as much as Corfu by the filtering. We probably need something to distinguish backend-filter-string and completion-sty-filter-string. You are using my Consult async commands, consult-ripgrep etc? I mean something like this, #grep-regexp#completion-sty-filter. The backend handles only the first part, the frontend handles the second part. In the context of LSP you wrote that there is not even the notion of a filter string passed to the backend, so one would actually only have trigger-prefix#filter-string, and trigger-prefix# is then ignored by the completion styles. The user has to enter this like trigger-prefix TAB #filter-string.

@astoff
Copy link
Contributor

astoff commented Apr 24, 2021

Since isearch-mb was mentioned here at some point, let me say that I made some changes and I think it works quite well now.

I should work out of the box with any isearch extension that starts a search (e.g., consult-isearch), and it just needs an advice to integrate with commands that run during the search or end a search. For instance:

(advice-add 'consult-line :around 'isearch-mb--after-exit)
(define-key isearch-mb-minibuffer-map (kbd "M-s l) 'consult-line)

There's some flicker in the match counts, but this is solved in Emacs 28.

@minad
Copy link
Owner Author

minad commented Apr 24, 2021

@astoff Do you plan to push some of the changes upstream? I saw a mail on emacs devel.

@astoff
Copy link
Contributor

astoff commented Apr 25, 2021

Good question. I think this is good enough for a (M)ELPA package now (I'll test it some more, anyone else is welcome to try as well). I'm skeptical the functionality could make it into isearch.el (if that's what you meant by upstream), because the Emacs people are extremely conservative about Isearch (when I tried to fix Isearch's manifestation of this issue, they wanted to introduce a defcustom to enable the correct behavior!). But I guess I'll ask.

@minad
Copy link
Owner Author

minad commented Apr 25, 2021

@astoff It does not seem that conservative to me. From what I see there have been adjustments recently. Attaching the sorting properties to the strings for example, which was introduced with 27. But introducing an entirely different interaction mode is certainly more problematic. I don't know what the best way forward is. If its up to me I would probably make a radical change and throw out the existing isearch echo area mode. But I am not an Emacs maintainer and priest responsible for the preservation of all the Emacs idiosyncrasies.

@astoff
Copy link
Contributor

astoff commented Apr 25, 2021

In my comment, I referred exclusively to Isearch key sequences. Apparently they have to be strictly backwards compatible. Not even making isearch-del-char backtrack a bit, as you suggested in that issue, is acceptable 🤷

@minad
Copy link
Owner Author

minad commented Apr 27, 2021

Since dabbrev has been discussed here and @protesilaos talked a few times about dabbrev, I just made sure that Corfu works with it. See minad/corfu@fe8674a. Turns out, dabbrev does not provide a capf backend, but it still makes use of completion-in-region and the completion-in-region-function and therefore automatically works with Corfu 🎉

(@protesilaos But I assume you still want a more capable dabbrev command which shows longer completions...)

@protesilaos
Copy link
Sponsor

protesilaos commented Apr 27, 2021 via email

@minad
Copy link
Owner Author

minad commented Apr 27, 2021

I guess that to call it with just TAB instead of dabbrev-completion we
would need it to have a capf backend? Basically my ideal functionality
would merge as many candidates as possible from all relevant backends.

Yes, indeed.

By the way, as I am writing this I realised that Corfu can work with
'message-mode-hook' when either BBDB or EBDB is in use: it helps
complete contacts in the To/Cc/Bcc fields.

This is probably the command mail-abbrev-complete-alias, which uses completion-in-region. Then there is another etags command. You can grep the code bases for (completion-in-region to see all uses. The most important use case is certainly in the completion-at-point command, which invokes the capfs.

@minad
Copy link
Owner Author

minad commented Apr 27, 2021

But I must say, actually I find it quite handy to have dabbrev always available on a separate key with the same UI. It may still make sense to implement a dabbrev capf. Since dabbrev.el already contains a completion table used by dabbrev-completion, with a slight refactoring one could turn the functionality into a capf.

@oantolin
Copy link
Contributor

Weird, I can't get dabbrev-completion to use Corfu. If I turn corfu-mode off, dabbrev-completion does use consult-completion-in-region, but with corfu-mode on I get the "Scanning for dabbrevs...done" message in the echo area but nothing happens, the Corfu UI does not appear.

@minad
Copy link
Owner Author

minad commented Apr 27, 2021

@oantolin Maybe you use an outdated version since I fixed this just recently?

@oantolin
Copy link
Contributor

Probably, I'm using the one from GNU ELPA, not the main branch.

@oantolin
Copy link
Contributor

Yep, that was it. Current main works just fine, sorry for the noise.

@oantolin
Copy link
Contributor

@protesilaos Did you mean mail-abbrev-complete-alias as @minad thought? You mentioned BBDB, so I thought you might mean bbdb-complete-mail. If that's what you meant, I'd love for that to use Corfu, it always seems to popup a *Completions* buffer for me.

@minad
Copy link
Owner Author

minad commented Apr 27, 2021

bbdb-complete-mail does not use completion-in-region, it seems to do its own thing. See http://git.savannah.gnu.org/cgit/emacs/elpa.git/tree/lisp/bbdb-com.el?h=externals/bbdb#n2159

@oantolin
Copy link
Contributor

Yep, it saddens me. And mail-abbrev-complete-alias isn't very useful for me because I mostly don't use mail aliases. I think I only have one for each class I teach that expands to the list of all students in that class. For individual people I don't tend to use them.

@protesilaos
Copy link
Sponsor

protesilaos commented Apr 27, 2021 via email

@oantolin
Copy link
Contributor

capf! Great! I should switch to EBDB, I think this is one of its improvements over BBDB.

@protesilaos
Copy link
Sponsor

protesilaos commented Apr 27, 2021 via email

@oantolin
Copy link
Contributor

I've migrated to EBDB and the capf email address completion is very nice! Thanks @protesilaos. By the way, I found a typo in your dotemacs: there is one occurrence of "EDBD" (I found it by making the same typo: I searched for "EDBD" and was surprised to see only one hit!).

@protesilaos
Copy link
Sponsor

protesilaos commented Apr 27, 2021 via email

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

9 participants