Helm capf does not call exit-function #2265

yyoncho · 2020-01-20T12:54:52Z

Expected behavior

helm calls :exit-function

Actual behavior (from `emacs-helm.sh` if possible, see note at the bottom)

helm does not call exit function

Steps to reproduce (recipe)

(defun bug-completion-at-point ()
  (list
   (point)
   (point)
   (completion-table-dynamic (lambda (&rest _)
                               (list "foobar"
                                     "foobar2")))
   :exit-function (lambda (candidate _status)
                    (debug))))
(setq-local completion-at-point-functions (list 'bug-completion-at-point))

Describe versions of Helm, Emacs, operating system, etc.

Latest from melpa, emacs 27/26 . Linux.

Are you using `emacs-helm.sh` to reproduce this bug? (yes/no):

No, but reproduced in clean env with

Are you using Spacemacs? (yes/no):

No.

The text was updated successfully, but these errors were encountered:

thierryvolpiatto · 2020-01-20T14:33:37Z

Ivan Yonchovski <notifications@github.com> writes:

Expected behavior helm calls :exit-function

No.

helm does not call exit function

:exit-function is only needed for the emacs vanilla completion UI, i.e. "*completions*" buffer, so yes helm doesn't handle it, though it handle :annotation-function. What do you want do do exactly?

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

yyoncho · 2020-01-20T14:38:09Z

:exit-function is used by the backends (capf functions) to perform post-completion processing(e. g. expanding snippets). ATM the only frontend that does not call :exit-function is helm. It works fine with company-capf, ivy and vanilla. The result is that helm capf frontend does not work with lsp-mode(and probably all other backends that perform post-completion).

thierryvolpiatto · 2020-01-20T16:38:41Z

Ivan Yonchovski <notifications@github.com> writes:

:exit-function is used by the backends (capf functions) to perform post-completion processing(e. g. expanding snippets). ATM the only frontend that does not call :exit-function is helm. It works fine with company-capf, ivy and vanilla. The result is that helm capf frontend does not work with lsp-mode(and probably all other backends that perform post-completion).

Thanks for explanation, is this working for you? diff --git a/helm-mode.el b/helm-mode.el index cb75b989..0dd9e32c 100644 --- a/helm-mode.el +++ b/helm-mode.el @@ -1674,7 +1674,8 @@ Can be used for `completion-in-region-function' by advicing it with an (advice-add 'lisp--local-variables :around #'helm-mode--advice-lisp--local-variables) - (let ((old--helm-completion-style helm-completion-style)) + (let ((old--helm-completion-style helm-completion-style) + string) (helm-aif (cdr (assq major-mode helm-completion-styles-alist)) (customize-set-variable 'helm-completion-style (if (cdr-safe it) (car it) it))) @@ -1682,7 +1683,7 @@ Can be used for `completion-in-region-function' by advicing it with an (let* ((enable-recursive-minibuffers t) (completion-flex-nospace t) (completion-styles (helm--prepare-completion-styles)) - (input (buffer-substring-no-properties start end)) + (input (setq string (buffer-substring-no-properties start end))) ;; Always start with prefix to allow completing without ;; the need of inserting a space after cursor or ;; relaying on crap old completion-styles emacs22 which @@ -1818,6 +1819,7 @@ Can be used for `completion-in-region-function' by advicing it with an t) ; exit minibuffer immediately. :must-match require-match)))) (helm-completion-in-region--insert-result result start point end base-size)) + (completion--done string 'finished) (customize-set-variable 'helm-completion-style old--helm-completion-style) (setq helm-completion--sorting-done nil) (advice-remove 'lisp--local-variables

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

thierryvolpiatto · 2020-01-20T17:29:13Z

If you can provide a recipe on how to reproduce with lsp-mode it is perfect ;-)
At least with the recipe you sent and the patch above applied, the exit fn is called.

Note: I know nothing about lsp-mode and how to make it working (dependencies etc..).
Thanks.

yyoncho · 2020-01-20T18:42:08Z

Thank you for the quick reply, I will test it on my side and report back.

yyoncho · 2020-01-20T19:58:29Z

Using the patch from above I get an error in lsp-mode since it is called with candidate = "" (in your case the string param in completion--done). I would expect completion--done to be called with the selected candidate as it was generated from the capf function since we encode properties in it.

thierryvolpiatto · 2020-01-20T22:03:10Z

Ivan Yonchovski <notifications@github.com> writes:

Using the patch from above I get an error in lsp-mode since it is called with candidate = "" (in your case the string param in completion--done). I would expect completion--done to be called with the selected candidate as it was generated from the capf function since we encode properties in it.

Ok, thanks, here the modified patch. diff --git a/helm-mode.el b/helm-mode.el index cb75b989..3cdb81af 100644 --- a/helm-mode.el +++ b/helm-mode.el @@ -1674,7 +1674,8 @@ Can be used for `completion-in-region-function' by advicing it with an (advice-add 'lisp--local-variables :around #'helm-mode--advice-lisp--local-variables) - (let ((old--helm-completion-style helm-completion-style)) + (let ((old--helm-completion-style helm-completion-style) + string) (helm-aif (cdr (assq major-mode helm-completion-styles-alist)) (customize-set-variable 'helm-completion-style (if (cdr-safe it) (car it) it))) @@ -1817,7 +1818,9 @@ Can be used for `completion-in-region-function' by advicing it with an (message "[No matches]"))) t) ; exit minibuffer immediately. :must-match require-match)))) + (setq string result) (helm-completion-in-region--insert-result result start point end base-size)) + (completion--done string 'finished) (customize-set-variable 'helm-completion-style old--helm-completion-style) (setq helm-completion--sorting-done nil) (advice-remove 'lisp--local-variables

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

yyoncho · 2020-01-21T08:59:03Z

I tested the last patch - the candidate text is the correct one but the text properties are stipped.

thierryvolpiatto · 2020-01-21T18:52:53Z

Ivan Yonchovski <notifications@github.com> writes:

I tested the last patch - the candidate text is the correct one but the text properties are stipped.

Yes text properties are always stripped, at least in helm-mode. So what you are expecting is much harder to fix (I mean to fix without breaking everything else).

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

* helm-mode.el (helm--completion-in-region): Call completion--done at end.

yyoncho · 2020-01-21T20:01:36Z

Isn't it possible to fix only this source to provide a lookup and allow finding the original item without changing the helm core stuff? Another option is to do that on our side via hacky buffer-local variable solution to do the same. And the third option is to declare/document both helm capf and lsp-mode incompatible.

thierryvolpiatto · 2020-01-22T04:48:46Z

Ivan Yonchovski <notifications@github.com> writes:

Isn't it possible to fix only this source to provide a lookup and allow finding the original item without changing the helm core stuff?

Kind of, this is what I am working on.

…

-- Thierry Get my Gnupg key: gpg --keyserver pgp.mit.edu --recv-keys 59F29997

* helm-mode.el (helm-completion-in-region-selection): New. (helm--completion-in-region): Use it in helm-before-action-hook.

thierryvolpiatto · 2020-01-22T06:07:19Z

Can you try last changes?

thierryvolpiatto · 2020-01-22T07:53:42Z

Merged to master.

yyoncho · 2020-01-22T08:08:55Z

I can confirm that it works fine. Thank you.

Allow running extra property :exit-function (Issues #2265, #2356). Passing 'finished to `completion--done' is probably the more logical as completion is always finished in our case when pressing RET, at one point 'finished was addind a space at insertion so we used 'exact, but it seems it is no more the case.

thierryvolpiatto pushed a commit that referenced this issue Jan 21, 2020

Honor :exit-function in completion-in-region (#2265)

203dbe8

* helm-mode.el (helm--completion-in-region): Call completion--done at end.

thierryvolpiatto pushed a commit that referenced this issue Jan 22, 2020

Don't loose properties in completion-in-region (#2265)

8ed26cb

* helm-mode.el (helm-completion-in-region-selection): New. (helm--completion-in-region): Use it in helm-before-action-hook.

thierryvolpiatto added the bug label Jan 22, 2020

yyoncho closed this as completed Jan 22, 2020

yyoncho mentioned this issue Jan 24, 2020

[Spacemacs.develop] [lsp-go] No autocompletion with lsp-go, autocompletion OK in plain go-mode emacs-lsp/lsp-mode#1344

Closed

clemera mentioned this issue May 4, 2020

Utilize completion metadata and exit-function in completion-in-region-function radian-software/selectrum#84

Merged

thierryvolpiatto mentioned this issue Aug 4, 2020

Call :exit-function with 'finished when completion is finished (currently 'exact) #2356

Closed

yyoncho mentioned this issue Sep 6, 2020

lsp-completion--exit-fn: works with non props candidate emacs-lsp/lsp-mode#2146

Merged

thierryvolpiatto mentioned this issue Dec 27, 2020

Helm adds space after completing directory name in M-x shell #2274

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Helm capf does not call exit-function #2265

Helm capf does not call exit-function #2265

yyoncho commented Jan 20, 2020

thierryvolpiatto commented Jan 20, 2020 via email

yyoncho commented Jan 20, 2020

thierryvolpiatto commented Jan 20, 2020 via email

thierryvolpiatto commented Jan 20, 2020

yyoncho commented Jan 20, 2020

yyoncho commented Jan 20, 2020

thierryvolpiatto commented Jan 20, 2020 via email

yyoncho commented Jan 21, 2020

thierryvolpiatto commented Jan 21, 2020 via email

yyoncho commented Jan 21, 2020

thierryvolpiatto commented Jan 22, 2020 via email

thierryvolpiatto commented Jan 22, 2020

thierryvolpiatto commented Jan 22, 2020

yyoncho commented Jan 22, 2020

Helm capf does not call exit-function #2265

Helm capf does not call exit-function #2265

Comments

yyoncho commented Jan 20, 2020

Expected behavior

Actual behavior (from emacs-helm.sh if possible, see note at the bottom)

Steps to reproduce (recipe)

Describe versions of Helm, Emacs, operating system, etc.

Are you using emacs-helm.sh to reproduce this bug? (yes/no):

Are you using Spacemacs? (yes/no):

thierryvolpiatto commented Jan 20, 2020 via email

yyoncho commented Jan 20, 2020

thierryvolpiatto commented Jan 20, 2020 via email

thierryvolpiatto commented Jan 20, 2020

yyoncho commented Jan 20, 2020

yyoncho commented Jan 20, 2020

thierryvolpiatto commented Jan 20, 2020 via email

yyoncho commented Jan 21, 2020

thierryvolpiatto commented Jan 21, 2020 via email

yyoncho commented Jan 21, 2020

thierryvolpiatto commented Jan 22, 2020 via email

thierryvolpiatto commented Jan 22, 2020

thierryvolpiatto commented Jan 22, 2020

yyoncho commented Jan 22, 2020

Actual behavior (from `emacs-helm.sh` if possible, see note at the bottom)

Are you using `emacs-helm.sh` to reproduce this bug? (yes/no):