cider icon indicating copy to clipboard operation
cider copied to clipboard

completion at point ignores namespace prefix

Open jaor opened this issue 3 years ago • 7 comments

Expected behavior

completion works as usual, across imported namespaces

Actual behavior

when calling completion-at-point, only completions belonging to the current namespace are offered (after C-c C-k to compile and load the namespace). this started happening very recently.

Steps to reproduce the problem

for me, it fails with any namespace i try. just C-c C-k and then M-x completion-at-point with point after a ':as' prefix. the prefix seems to be ignored, and only current namespace names are offered.

Environment & Version information

CIDER version information

;; CIDER 1.2.0snapshot (package: 20210524.832), nREPL 0.8.3
;; Clojure 1.10.1, Java 11.0.11

Lein/Boot version

Lein 2.9.1

Emacs version

27.2 (but also happens with 28)

Operating system

Debian unstable

jaor avatar May 24 '21 22:05 jaor

This code hasn't been changed in years, so I'm struggling to see what might have caused it for you. It'd be useful if you added some troubleshooting info here - e.g. the nREPL message exchange log.

bbatsov avatar May 25 '21 08:05 bbatsov

On Tue, May 25 2021, Bozhidar Batsov wrote:

This code hasn't been changed in years, so I'm struggling to see what might have caused it for you. It'd be useful if you added some troubleshooting info here - e.g. the nREPL message exchange log.

Hi Bozhidar... i'm struggling too. After discarding emacs 28, i at first thought it could be related to using corfu instead of company for region completions, but disabling both doesn't make any difference, and i cannot think of anything directly related in my config that might have changed. I was hoping someone else was having the same problem. I'll try to find a short reproducer. Thanks for chiming in!

jaor avatar May 25 '21 13:05 jaor

i think i've found the cause of the problem. i was playing with completion-styles. its default is '(basic partial-completion emacs22). if one changes it to, say, '(partial-completion) (or, in my case, '(orderless)), completion stops working. it seems basic is needed (or something similar). fwiw, other capf like eshell's or elisp's don't seem to have this limitation. things also work if basic is combined with other styles (e.g., i'm using now '(orderless basic) without trouble).

jaor avatar May 25 '21 19:05 jaor

Might be related to https://github.com/clojure-emacs/cider/issues/3006

bbatsov avatar Aug 29 '21 07:08 bbatsov

Just ran into the exact same problem. Confirmed on Emacs 28.0.50; cider 1.2.0 I had completion-styles set to '(orderless) and completion would error with out of range when cider-complete attempted to put-property. That's because even though prefix is correctly identified by checking bounds inside cider-completion-at-point, cider-complete receives an empty prefix. No idea why.

First tried this but had no effect: (edit: it does starting from https://github.com/clojure-emacs/cider/pull/3226)

(setq completion-category-overrides '((cider (styles basic)))

This however gets my completions back:

(defun my-completion-at-point ()
   (interactive)
   (let ((completion-styles '(basic)))
       (call-interactively #'completion-at-point)))

Completions work as expected everywhere else e.g. in emacs-lisp buffers.

vkz avatar Jan 29 '22 12:01 vkz

There's also potential issue with vertico completions. I'm just going to mention it here in case someone figures if it isn't cider's fault after all. Naturally basic and partial completions won't cut it for symbols, but if in the above hacky my-completion-at-point you set (completion-styles '(basic flex)) or (basic orderless) upon completion (guessing when basic style fails) vertico reports out-of-range error in post-command, specifically as it attempts to render candidades with vertico--exibit.

vkz avatar Jan 29 '22 14:01 vkz

I was experiencing this same issue and I dug into it and think I found the root problem. This issue in particular is caused by the orderless completion style. As mentioned above, the cider completion function is not getting the correct prefix. The reason is that the orderless completion (fitting with its name) doesn't enforce an order with the completion string. So for example "def abc" could end up matching "abcxyzdef". This is why the orderless completion style doesn't give cider complete the package prefix. In Orderless's world, Cider should return all of the completion candidates for every package, and then Orderless, when given a pattern of "str/" will narrow them all down, maybe for example it could match something like "test-str/". This doesn't work for cider however because (I assume) nrepl needs to know the prefix so it cat fetch all the identifiers in that particular package. I assume it wouldn't be feasible to fetch all possible completions for all required packages.

It also makes sense why other completion styles run into this same issue because they to cant specify a definitive prefix.

If you're curious, you can look at how the orderless-filter function works. Notice how it figures out the package prefix, but since the matching regex isn't "anchored", it doesn't say for sure that it is the prefix.

I was able to get orderless to work by providing it with the required functions to handle package prefixes. With the code below I was able to get orderless to work properly:

;; This is the function that breaks apart the pattern.  To signal that
;; an element is a package prefix, we keep its trailing "/" and return
;; the rest as another pattern.
(defun cider-orderless-component-separator (pattern)
  (if (cider-connected-p)
      (let ((slash-idx (string-match-p "/" pattern)))
        (if slash-idx
            (append (list (substring pattern 0 (1+ slash-idx)))
                    (split-string (substring pattern (1+ slash-idx)) " +" t))
          (split-string pattern " +" t)))
    (split-string pattern " +" t)))

;; This is the function that takes our package prefix and ensures that
;; it is at the beginning (note the "^" in the regex).
(defun cider-orderless-package-prefix (component)
  (format "\\(?:^%s\\)" (regexp-quote component)))

;; This is the function that notices that the candidate ends in a "/"
;; and that it should use our `cider-orderless-package-prefix'
;; function for turning the candidate into a regex.
(defun cider-package-prefix (pattern _index _total)
  (when (and (cider-connected-p) (string-suffix-p "/" pattern))
    'cider-orderless-package-prefix))

(defun cider-setup-orderless ()
  (setq orderless-style-dispatchers '(cider-package-prefix))
  (add-to-list 'orderless-matching-styles #'cider-orderless-package-prefix)
  (setq orderless-component-separator #'cider-orderless-component-separator))

I'd definitely be willing to clean this up and make a PR for this if it's determined that this is a desirable integration to have in Cider (though this would only solve for orderless). A more robust solution would probably involve some way to determine all possible completions across all requirements of a package and let the completion style work from that. I'm not sure how feasible this is.

zkry avatar Feb 04 '22 01:02 zkry

(defun mm/cider-complete-at-point ()
  "Complete the symbol at point."
  (when (and (cider-connected-p)
	     (not (cider-in-string-p)))
    (when-let*
	((bounds
	  (bounds-of-thing-at-point
	   'symbol))
	 (beg (car bounds))
	 (end (cdr bounds))
	 (completion
	  (append
	   (cider-complete
	    (buffer-substring beg end))
	   (get-text-property (point) 'cider-locals))))
      (list
       beg
       end
       (completion-table-dynamic
	(lambda (_) completion))
       :annotation-function #'cider-annotate-symbol))))

(advice-add 'cider-complete-at-point :override #'mm/cider-complete-at-point)

this is an alternative if you are interested. It just does what is needed.

I have (setf completion-styles '(orderless basic))

  • I don't remember right now if adding basic was neccessary or not.

benjamin-asdf avatar Nov 29 '22 09:11 benjamin-asdf

Implemented a separate orderless matching style to treat / as .* and slightly updated @benjamin-asdf's implementation to get a better out of the box orderless matching experience:

image image

https://git.sr.ht/~abcdw/rde/tree/f7f97d0/rde/features/clojure.scm#L91

It's a workaround, not a proper solution IMO, but works quite good.

abcdw avatar Dec 01 '22 15:12 abcdw

Probably the simplest resolution for this issue would be to document that orderless is typically undesired whenever cider-mode is enabled.

@zkry's patch seems reasonable - PR welcome (n.b. it's the kind of change that deserves good test coverage)

vemv avatar Aug 18 '23 18:08 vemv

CIDER, as of Git master and 20231009.804 snapshot declares:

(add-to-list 'completion-category-overrides '(cider (styles basic flex)))

As now documented in https://docs.cider.mx/cider/usage/code_completion.html#completion-styles

This ensures that one doesn't try to use completion styles that currently there's no support for, from our side, fixing various use cases.

(NOTE: this technique was observed not to work in a previous comment of this thread. That was true back then, but not anymore)

I'll close this issue. A new one named e.g. "Support orderless completion style" would be welcome provided that you intend to implement it.

vemv avatar Oct 09 '23 08:10 vemv

@vemv does it mean that flex completion style is now always enabled for CIDER by default? If so, how can I disable it? Should I override completion-category-overrides in cider-mode-hook?

rrudakov avatar Oct 09 '23 14:10 rrudakov

Looks like you missed this bit from the docs:

Note that flex will only work if you also enabled fuzzy candidate matching.

vemv avatar Oct 09 '23 14:10 vemv

Looks like you missed this bit from the docs:

Note that flex will only work if you also enabled fuzzy candidate matching.

I'm confused. Isn't there a difference between cider completion category and cider completion style? completion-category-overrides affects completion category and not completion style.

rrudakov avatar Oct 09 '23 14:10 rrudakov

I also found some aspects confusing, however things are working as intended now:

  • basic completion works
  • flex completion is optional and honors the setting CIDER has for it
  • orderless completion is disabled so users that favor it see it fall back to basic instead of seeing no completions at all

If there's a practical problem, please let us know.

vemv avatar Oct 09 '23 14:10 vemv

OK, thanks for the explanation. I'll try to test it carefully tomorrow.

rrudakov avatar Oct 09 '23 14:10 rrudakov

@vemv I tested it with the latest version of CIDER and as I expected flex is enabled by default even if fuzzy matching is not enabled: image

rrudakov avatar Oct 10 '23 09:10 rrudakov

Hmm, that's right.

We probably should only add flex dynamically from the cider-company-enable-fuzzy-completion defun. Let me try it.

...There's also the question of whether cider-company-enable-fuzzy-completion makes sense in modern-day Emacs. flex was introduced in Emacs 27 (3 years after cider-company-enable-fuzzy-completion was introduced).

vemv avatar Oct 10 '23 09:10 vemv

Keep in mind we still need to support older Emacsen, though. (in particular Emacs 26, at least until CIDER 2.0)

bbatsov avatar Oct 10 '23 11:10 bbatsov

Correct - I appreciate for the confirmation! My WIP has a soft deprecation only. PR soon.

vemv avatar Oct 10 '23 11:10 vemv