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

completion at point ignores namespace prefix #3019

Closed
jaor opened this issue May 24, 2021 · 20 comments
Closed

completion at point ignores namespace prefix #3019

jaor opened this issue May 24, 2021 · 20 comments
Labels
bug good first issue A simple tasks suitable for first-time contributors help wanted

Comments

@jaor
Copy link
Contributor

jaor commented May 24, 2021

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

@bbatsov
Copy link
Member

bbatsov commented May 25, 2021

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.

@jaor
Copy link
Contributor Author

jaor commented May 25, 2021 via email

@jaor
Copy link
Contributor Author

jaor commented May 25, 2021

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).

@bbatsov bbatsov added bug good first issue A simple tasks suitable for first-time contributors help wanted labels Jun 17, 2021
@bbatsov
Copy link
Member

bbatsov commented Aug 29, 2021

Might be related to #3006

@vkz
Copy link

vkz commented Jan 29, 2022

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 #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
Copy link

vkz commented Jan 29, 2022

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.

@zkry
Copy link
Contributor

zkry commented Feb 4, 2022

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.

@benjamin-asdf
Copy link
Contributor

(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.

@abcdw
Copy link

abcdw commented Dec 1, 2022

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.

schmir added a commit to schmir/.emacs.d that referenced this issue Jan 15, 2023
This breaks cider's completion for namespaced symbols for me.
See clojure-emacs/cider#3019
@vemv
Copy link
Member

vemv commented Aug 18, 2023

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 vemv pinned this issue Sep 21, 2023
@vemv vemv unpinned this issue Oct 9, 2023
@vemv
Copy link
Member

vemv commented Oct 9, 2023

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 vemv closed this as completed Oct 9, 2023
@rrudakov
Copy link
Contributor

rrudakov commented Oct 9, 2023

@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?

@vemv
Copy link
Member

vemv commented Oct 9, 2023

Looks like you missed this bit from the docs:

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

@rrudakov
Copy link
Contributor

rrudakov commented Oct 9, 2023

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.

@vemv
Copy link
Member

vemv commented Oct 9, 2023

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.

@rrudakov
Copy link
Contributor

rrudakov commented Oct 9, 2023

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

@rrudakov
Copy link
Contributor

@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

@vemv
Copy link
Member

vemv commented Oct 10, 2023

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).

Update

Fixed in 36737a9

@bbatsov
Copy link
Member

bbatsov commented Oct 10, 2023

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

@vemv
Copy link
Member

vemv commented Oct 10, 2023

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

@clojure-emacs clojure-emacs locked as resolved and limited conversation to collaborators May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug good first issue A simple tasks suitable for first-time contributors help wanted
Projects
None yet
Development

No branches or pull requests

8 participants