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

:exit-function called with 'exact STATUS when completion-at-point called on already completed sole completion candidate #2646

Closed
1 task done
non-Jedi opened this issue Mar 18, 2024 · 9 comments
Labels

Comments

@non-Jedi
Copy link

What happened?

When completing fo to foo, helm correctly calls the :exit-function with status 'finished, but when completing foo to foo, status is instead 'exact. This contradicts the behavior specified by the docstring for completion-extra-properties:

:exit-function: Function to run after completion is performed.

   The function must accept two arguments, STRING and STATUS.
   STRING is the text to which the field was completed, and
   STATUS indicates what kind of operation happened:
     finished - text is now complete
     sole     - text cannot be further completed but
                  completion is not finished
     exact    - text is a valid completion but may be further
                  completed.

According to this docstring, status should be 'finished. This bug is similar to #2356 but more specific. I can also confirm that both emacs native completion and corfu will pass status as 'finished in this case.

This is important for julia mode per JuliaEditorSupport/julia-emacs#204 since julia-mode uses :exit-function to for example substitute "λ" into a buffer when "\lambda" is completed.

Actual emacs version is 29.2 but that wasn't available in dropdown menu.

How to reproduce?

Evaluate:

(defun my-capf ()
  (list (line-beginning-position) (point) '("foo" "bar")
        :exit-function (lambda (_string status)
                         (message ":exit-function status: %s" status))))

(setq completion-at-point-functions (list 'my-capf))

Type "foo" and press C-M-i. Message will appear: ":exit-function status: exact".

Helm Version

Master branch

Emacs Version

Emacs-29.1

OS

GNU/Linux

Relevant backtrace (if possible)

No response

Minimal configuration

  • I agree using a minimal configuration
@non-Jedi non-Jedi added the bug label Mar 18, 2024
@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Mar 18, 2024 via email

@non-Jedi
Copy link
Author

Hi Thierry,

Thanks for working with me on this again.

We already fixed this in Bug#2356 and at the time it was ok for you,
what have changed since then?

Your solution for bug #2356 fixed completion of "fo" to "foo". I did not realize at the time that it did not fix completion of "foo" to "foo". Nothing changed but the extent of my own ignorance.

I can change again the behavior of this if needed, I have personally no
opinions on what to do with this as it is still badly documented, here
what I would like to know:

  1. When try-completions returns t, finished or exact?
  2. When try-completions returns a string, finished or exact?
  1. This should always be 'finished.
  2. To answer this, I need to better understand when helm calls the :exit-function:

If the :exit-function is called when completing "fo" to "foo" but not exiting helm completion because "foobar" is also an option, then the :exit-function should be called with 'exact in that case and 'finished when the user selects "foo" as the final completion. But if the :exit-function is only ever called when completion is successfully finished, then it can just be called with 'finished every time regardless of what try-completions returns.

I would tend to prefer 'finished for 1) and 'exact for 2), it is more
logic, I am not sure why we choose to use the contrary at the time.

I am not sure either. I admit that I didn't closely enough read your comment or the code at the time to understand when you were using 'exact versus 'finished.

@non-Jedi
Copy link
Author

From how I understand helm working, I don't think you ever do partial completion (e.g. completing "fo" to "foo" before the user has selected "foo" as the final completion), so I think 'finished is all you need.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Mar 19, 2024 via email

@non-Jedi
Copy link
Author

5a5bdbf just reverses the problem. Now I get 'finished when completing "foo" to "foo" but not when completing "fo" to "foo". It should happen for both. When I get a chance, I will try to take a look at the other functions referenced in code comments; I don't understand why 'exact would be needed under helm's completion paradigm.

Base emacs needs 'exact because it will complete "f" to "fo" given completion candidate "foo" and "fob", but helm doesn't do intermediate completion.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Mar 20, 2024 via email

@non-Jedi
Copy link
Author

Ok, I will use 'exact only when the string returned by try-completions ends with "/" until we find something better.

Thanks! Can confirm that eae4f5a fixes the issue for me.

However why don't you use like eglot does something like this in your capf:

  (lambda (proxy status)
    (when (memq status '(finished exact))
      [...]))

This causes an issue for other completion frameworks that do partial completion. Let's say someone is trying to enter "\leftarrowless" which our :exit-function would replace with "⥺":

  1. User enters "\leftar" and invokes completion-at-point.
  2. "\leftar" partially completes to "\leftarrow", and the :exit-function is called with status 'exact.
  3. :exit-function replaces "\leftarrow" with "←" leaving the user confused and starting over again to input "⥺"

That's why we have to filter only on 'finished.

@non-Jedi
Copy link
Author

Still need to figure out a proper solution eventually instead of just checking for string ending in a slash. We have exactly one completion candidate ending with a slash: "\1/" => "⅟". But at this point, I think the onus is squarely on me to figure it out.

Note to self for when I return to this in the future: the problem being solved by the check for string ending in "/" is Bug #2274.

@thierryvolpiatto
Copy link
Member

thierryvolpiatto commented Mar 28, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants