minad / cape

🦸cape.el - Completion At Point Extensions
GNU General Public License v3.0
584 stars 20 forks source link

orderless-prefixes and cape-super-capf not working together #45

Closed jdtsmith closed 2 years ago

jdtsmith commented 2 years ago

CAPFs composed with cape-super-capf seem to be defeated by the orderless-prefixes or orderless-initialism styles. From emacs -Q:

;; --- Eval these
(package-initialize)
(require 'orderless)
(require 'cape)

(setq completion-at-point-functions (list #'elisp-completion-at-point)
      completion-styles '(orderless)
      completion-category-defaults nil
      orderless-matching-styles '(orderless-literal))
;; -- end eval

;; # 1
(completion--cat ; M-Tab: -> completion--category-override, literal working!
 )

;; Now try cape with Dabbrev
completion--cats-are-cool ;; <-- a dabbrev target
(setq completion-at-point-functions
      (list (cape-super-capf #'elisp-completion-at-point #'cape-dabbrev)))

;; # 2
(completion--cat ; M-Tab: now includes dabbrev result! Cape+literal is working!
 )

;; Now try orderless-prefixes (no cape)
(setq completion-at-point-functions (list #'elisp-completion-at-point)
      orderless-matching-styles '(orderless-prefixes))

;; # 3
(com--cat ; M-Tab: expands to completion--category-override, prefixes working!
 )

;; Now try cape and prefixes together
(setq completion-at-point-functions
      (list (cape-super-capf #'elisp-completion-at-point #'cape-dabbrev)))

;; # 4
(com--cat ; M-Tab: nothing -- cape+prefixes not working :(
 )
jdtsmith commented 2 years ago

Thanks hadn't noticed this fix. With it, I do now get results from the super-capf with prefix (last M-Tab above), but they do not ever include dabbrev results.

minad commented 2 years ago

I get dabbrev results. Note that the prefix must match literally for dabbrev.

jdtsmith commented 2 years ago

You get both flavors of candidates on the last com--cat (# 4) above (I added numbers for clarity)?

Just updated to today's cape and 0527 orderless, and now have the following error from emacs -Q for completion # 2, with orderless-literal style and the super-capf.

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  orderless--highlight((#("\\(?:\\(?:\\(\\bcompletion\\).*\\(\\b--cat\\).*\\(\\b\\)\\)\\)" 12 22 (fontified t) 30 35 (fontified t))) nil)
  orderless-highlight-matches(#("completion--cat" 0 15 (fontified t)) ("completion--category-override" "completion--cats-are-cool" nil))
  orderless-all-completions(#("completion--cat" 0 15 (fontified t)) #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil 15)
  #f(compiled-function (style) #<bytecode -0x2252f377bd23a4e>)(orderless)
  completion--some(#f(compiled-function (style) #<bytecode -0x2252f377bd23a4e>) (orderless))
  completion--nth-completion(2 #("completion--cat" 0 15 (fontified t)) #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil 15 (metadata (category . cape-super) (display-sort-function . identity) (cycle-sort-function . identity)))
  completion-all-completions(#("completion--cat" 0 15 (fontified t)) #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil 15 (metadata (category . cape-super) (display-sort-function . identity) (cycle-sort-function . identity)))
  minibuffer-completion-help(#<marker at 713 in *scratch*> 728)
  completion--do-completion(#<marker at 713 in *scratch*> 728)
  completion--in-region-1(#<marker at 713 in *scratch*> 728)
  #f(compiled-function (start end collection predicate) #<bytecode -0xafcd704afbb2c30>)(#<marker at 713 in *scratch*> 728 #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil)
  apply(#f(compiled-function (start end collection predicate) #<bytecode -0xafcd704afbb2c30>) (#<marker at 713 in *scratch*> 728 #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil))
  #f(compiled-function (funs global args) #<bytecode -0x1b5fef248792b22>)(nil nil (#<marker at 713 in *scratch*> 728 #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil))
  completion--in-region(#<marker at 713 in *scratch*> 728 #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil)
  completion-in-region(#<marker at 713 in *scratch*> 728 #f(compiled-function (str pred action) #<bytecode 0x1843347f30a27f21>) nil)
  completion-at-point()
  funcall-interactively(completion-at-point)
  command-execute(completion-at-point)

Strangely, I again don't get any candidates with prefix (# 4), though no error.

minad commented 2 years ago

Strangely, I again don't get any candidates with prefix (# 4), though no error.

Try (completion--cat M-TAB.

jdtsmith commented 2 years ago

nil fix worked, thank you!

Strangely, I again don't get any candidates with prefix (# 4), though no error.

Try (completion--cat M-TAB.

Yes that works, but that's effectively literal completion, not prefix completion. Notice # 3, which (with orderless-prefix), completes com--cat correctly to completion--category-override (as long as cape isn't in the loop).

minad commented 2 years ago

Yes that works, but that's effectively literal completion, not prefix completion. Notice # 3, which (with orderless-prefix), completes com--cat correctly to completion--category-override (as long as cape isn't in the loop).

Dabbrev performs prefix expansion when it generates candidates, so the prefix is interpreted literally. I already mentioned this above.

jdtsmith commented 2 years ago

Dabbrev performs prefix expansion when it generates candidates, so the prefix is interpreted literally.

Yes. But this does not explain why the candidate from elisp-completion-at-point, which has no such constraint and works normally with orderless-prefix outside of the super-capf, gets excluded.

minad commented 2 years ago

Yes. But this does not explain why the candidate from elisp-completion-at-point, which has no such constraint and works normally with orderless-prefix outside of the super-capf, gets excluded.

I don't see that or I don't understand what you mean. In my tests this works all as expected. Maybe the prefix length check is what bites you here? The prefixes must have the same length. Company has the same limitation.

https://github.com/minad/cape/blob/86a1df6cbacd0e05b801208f83be5ca6c1cc8cec/cape.el#L532-L542

jdtsmith commented 2 years ago

I mean only that this final test:

;; Now try cape and prefixes together
(setq completion-at-point-functions
      (list (cape-super-capf #'elisp-completion-at-point #'cape-dabbrev)))

;; # 4
(com--cat ; M-Tab: nothing -- cape+prefixes not working :(
 )

returns no candidates. The prefix test part seems fine, and passes. I wonder if this:

https://github.com/minad/cape/blob/86a1df6cbacd0e05b801208f83be5ca6c1cc8cec/cape.el#L573-L583

is the issue? I note that neither table (dabbrev: expected, elisp: not expected) returns non-nil for the initial try-completion action. I also noticed that the elisp-capf by itself is called from completion-try-completions, but when part of the super-capf, gets called from from plain try-completion (which obviously won't respect orderless styles).

Poking a bit more I think it all comes back to this nugget in completion--capf-wrapper from minibuffer.el:

;; FIXME: Here we'd need to decide whether there are
               ;; valid completions against the current text.  But this depends
               ;; on the actual completion UI (e.g. with the default completion
               ;; it depends on completion-style) ;-(
               ;; We approximate this result by checking whether prefix
               ;; completion might work, which means that non-prefix completion
               ;; will not work (or not right) for completion functions that
               ;; are non-exclusive.
               (null (try-completion (buffer-substring-no-properties
                                      (car res) (point))
                                     (nth 2 res)
                                     (plist-get (nthcdr 3 res) :predicate)))

(Light go off)... If you are using corfu and not emacs -Q to test this, then your workaround for this is probably what's fixing it! The joy of rediscovery. Somebody should probably fix that FIXME :).

jdtsmith commented 2 years ago

Perhaps it's safer to leave :exclusive alone in cape-super-capf, until this misfeature is corrected.

minad commented 2 years ago

Ah yes, it works in Corfu. I didn't test default completion but I doubt that there is much interest in using default completion with Cape or cape-super-capf.

The problem with fixing that FIXME is that Stefan Monnier disagrees with me about how this should be fixed. He argues that if you use flexible completion styles (orderless, flex, etc.), using completion-try-completion in completion--capf-wrapper will make it too easy for the first Capf to take over. Therefore the wrapper intentionally tests only prefix completion to give the Capfs with lower priority a better chance to run. This argument is understandable, but I think it leads to more confusion (as in our case here). I rather prefer to rely on completion styles only. There is another technical difficulty - since the wrapper is agnostic to the completion UI and a completion UI may decide to not use completion styles (e.g. Ivy, Selectrum+Prescient, ...), the wrapper cannot use completion-try-completion. It would need an indirection, some hook. In Corfu all these issues are not relevant (and Corfu uses completion styles), so I can just fix it there in the way I deem correct. Furthermore Corfu intentionally only supports Capfs with the "new" (10 year old) calling convention. Legacy Capfs are not supported while completion--capf-wrapper has messy logic to somehow handle them.

minad commented 2 years ago

Stefan also argues that one should avoid :exclusive no because of this FIXME and also because he doesn't like the semantics of it. I also disagree with that. I find nothing wrong with trying one Capf and then fallback to another. By using :exclusive no such a fallback happens even if the first Capf could in principle complete in the given context. This means by using :exclusive no one relies completely on the decision of the Capfs to complete in a certain context. But this makes them far less composable and makes it harder to enhance completion with additional Capfs as we are trying here in Cape.

On the other hand :exclusive no is also more messy since it doesn't bother so much about the context of the completion but rather about the input of the user. If the user wants something else, then the next Capf is tried if no candidates match. Exclusive Capfs have the philosophy that they control everything, e.g., like lsp-mode tries to do.

There are two ways out if one wants to use the Capf fallback mechanism:

jdtsmith commented 2 years ago

Wow, a complex history.

Perhaps we could just make a cape-some-capf which composes various capfs similarly to how :exclusive no does, but does not assign exclusive :no to the composed capf itself. The user could decide how to compose: give me everything together, or "first returning candidates wins". So like (cape-some-capf (cape-super-capf #'elisp-completion-at-point #'cape-dabbrev) #'cape-file). This should work without the corfu workaround. Not all capf's can be super'd, but I suspect all could be some'd.

Use explicit keybindings per Capf

This is an interesting idea; I mean I already sometimes use and appreciate Dabbrev's binding (but I really appreciate dabbrev getting added to function name in corfu-auto, for symbols not yet in the obarray). Might be nicer if it were a transient, then you could have lots of capfs at your fingertips, and one "main capf" doing for auto (even better: than transient could help configure what the auto completion shows). In fact, I'd probably just bind M-Tab to such a complete transient. Then, almost nothing to remember.

I still find trouble with corfu completing a perfectly good word, but usually this is in comments/strings where file completion is allowed to occur. If I disable that, but have a quick "complete this as a file" keyword or transient-choice, that'd be ideal. The choices on the transient would be mode-dependent, and could as you say supplant the "narrowing" idea. So in python modes, complete "function", "module", "variable", "filename", "dabbrev", "line", "word", etc.

Then just add a little polish to let an active corfu session pop into the transient, reconfigure things, and re-spawn with the new candidates.

Such a system would be ... a true completion super-power.

minad commented 2 years ago

cape-some-capf ist not a bad idea. We would essentially transfer the fix from Corfu to Cape. However when would it be useful? Cape is mostly useful for Corfu users and they already get the fix. It would help Company and default UI users and add to the confusion. :)

I don't find the transient idea appealing. I am not the biggest fan of transients, which-key or org menu capture popups. However it would be nice if one could switch the Capf during completion. Unfortunately that's usually not easily possible, in particular not with Orderless. With prefix completion it already works, e.g. start with C-c p d and switch to C-c p k while preserving the prefix. For arbitrary prefixes one would have to delete the prefix first, start the new capf and then reinsert.

minad commented 2 years ago

We could still pursue the kind narrowing idea. I guess this is more what you are proposing? But this doesn't really relate to Capf composition.

jdtsmith commented 2 years ago

You may be right that non-corfu completion + cape is a pretty rare combo. I think cape-some-capf might be more straightforward to configure and reason about. But I guess in some ways it amounts to the same thing. One thing it would allow is arbitrary nesting ((some (super (some ...) (some... )) (super ...)), but that's got to be a rarity.

In an ideal world, CAPFs would be very smart and very narrowly targeted, and know exactly what they could complete where. Elisp does "OK" with this: var vs. function positions for example. Some LSP servers are pretty smart about this too. But most capfs are pretty indiscriminate (looking at you dabbrev).

The transient idea essentially merges the narrowing concept (show me a subset of a given CAPFs nominal candidates, e.g. based on kind), with the "switch to a new CAPF with a special binding" functionality into one.

For simplicity, you could also call "a CAPF that narrows down to company-kinds matching this pattern" a new CAPF, then it's just the matter of seamlessly switching capfs mid-stream.

Maybe a cape-with-kind-predicate convenience could help there, then cape/corfu/etc. are out of the business of knowing what kinds will occur. The user can configure a few obvious targets as customized CAPFs themselves. lsp-only-variables, etc.

I don't understand why the prefix needs removing and reinserting. If you call the new desired capf while another is active, and it returns nil, an error/warning could be signaled.