completion discrepancy between default completion and helm/ivy completions

completion discrepancy between default completion and helm/ivy completions

[Alan Schmitt]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

Hello,

I'm tracking down a bug with org-contacts during completion, and I have
found the source to be a discrepancy between how default completion is
computed (using `completion-basic-try-completions') and how ivy/helm
completion is computed (using `completion-basic-all-completions').

In the first case, the result of the completion function is tested to be
a string, and tweaked if it is:

        (let ((completion (try-completion beforepoint table pred)))
          (if (not (stringp completion))
              completion
            (cons
             (concat completion
                     (completion--merge-suffix completion point afterpoint))
             (length completion))))

In the all-completions case, there is no such test, so the completion is
returned as such.

In org-contacts, the completion is defined as a function that returns is
a string, so it works with default completion but not with ivy or helm.

I would like to fix org-contacts so that it works well all the time.
Should it directly return a cons of the string and its length?

Thanks a lot for any help, and if this is not the place to ask such
questions, please let me know the right list.

Best,

Alan

-- 
OpenPGP Key ID : 040D0A3B4ED2E5C7
Monthly Athmospheric CO₂, Mauna Loa Obs. 2016-10: 401.57, 2015-10: 398.29

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]

Re: completion discrepancy between default completion and helm/ivy completions

[Stefan Monnier]

>         (let ((completion (try-completion beforepoint table pred)))
>           (if (not (stringp completion))
>               completion

This case is for when `completion` is t (when means that `beforepoint`
is a valid candidate and there is no further completion possible).

> I would like to fix org-contacts so that it works well all the time.
> Should it directly return a cons of the string and its length?

No, returning such a cons would be a bug and I'm pretty sure it would
break the default completion code at least.


        Stefan

Re: completion discrepancy between default completion and helm/ivy completions

[Alan Schmitt]

[-- Attachment #1: Type: text/plain, Size: 1141 bytes --]

Hello Stefan,

On 2016-11-10 15:35, Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>         (let ((completion (try-completion beforepoint table pred)))
>>           (if (not (stringp completion))
>>               completion
>
> This case is for when `completion` is t (when means that `beforepoint`
> is a valid candidate and there is no further completion possible).

I see.

>> I would like to fix org-contacts so that it works well all the time.
>> Should it directly return a cons of the string and its length?
>
> No, returning such a cons would be a bug and I'm pretty sure it would
> break the default completion code at least.

OK, but I do not know how to proceed then.

Is it correct to register as a completion something that returns a
string? Because it is what org-contacts does, but although it works with
the default completion code, it breaks ivy or helm style completions.
I'm trying to understand if the bug lies within org-contacts or within
iyv/helm.

Thanks,

Alan

-- 
OpenPGP Key ID : 040D0A3B4ED2E5C7
Monthly Athmospheric CO₂, Mauna Loa Obs. 2016-10: 401.57, 2015-10: 398.29

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]

Re: completion discrepancy between default completion and helm/ivy completions

[Stefan Monnier]

> Is it correct to register as a completion something that returns a
> string?

I don't understand the question: register where?  return a string in
which case?

> Because it is what org-contacts does, but although it works with
> the default completion code, it breaks ivy or helm style completions.

AFAIK those frameworks probably don't handle well completion tables
which use completion-boundaries, so I'd lean towards saying it's a bug
or a known limitation of those frameworks.  But I haven't looked at them
in a long while, so maybe this isn't true any more.


        Stefan

Re: completion discrepancy between default completion and helm/ivy completions

[Alan Schmitt]

[-- Attachment #1: Type: text/plain, Size: 2129 bytes --]

On 2016-11-13 10:38, Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Is it correct to register as a completion something that returns a
>> string?
>
> I don't understand the question: register where?  return a string in
> which case?

Thank you for bearing with me, here are the details.

org-contacts adds completions hooks to message mode:

(when (and org-contacts-enable-completion
	   (boundp 'completion-at-point-functions))
  (add-hook 'message-mode-hook 'org-contacts-setup-completion-at-point))

which does

(defun org-contacts-setup-completion-at-point ()
  "Add `org-contacts-message-complete-function' as a new function
to complete the thing at point."
  (add-to-list 'completion-at-point-functions
	       'org-contacts-message-complete-function))

That function basically checks if this is a place where completion
should happen (To field, for instance), then calls all the
org-contacts-complete-functions until one succeeds. One such function is
org-contacts-complete-group that, in some case, returns a function
taking unused arguments and returning the completion as a string. To
summarize, is it okay for a function in completion-at-point-functions to
return such a function? The docstring seems to say it's OK (except the
function returned should not have any argument), but discouraged. I
guess I should try to return something of the form (start end collection
. props) instead, with collection being a singleton…

>> Because it is what org-contacts does, but although it works with
>> the default completion code, it breaks ivy or helm style completions.
>
> AFAIK those frameworks probably don't handle well completion tables
> which use completion-boundaries, so I'd lean towards saying it's a bug
> or a known limitation of those frameworks.  But I haven't looked at them
> in a long while, so maybe this isn't true any more.

Thank you for your help, I'm starting to understand what is going on.
I'll keep digging.

Best,

Alan

-- 
OpenPGP Key ID : 040D0A3B4ED2E5C7
Monthly Athmospheric CO₂, Mauna Loa Obs. 2016-10: 401.57, 2015-10: 398.29

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]

Re: completion discrepancy between default completion and helm/ivy completions

[Stefan Monnier]

> (when (and org-contacts-enable-completion
> 	   (boundp 'completion-at-point-functions))
>   (add-hook 'message-mode-hook 'org-contacts-setup-completion-at-point))

> which does

> (defun org-contacts-setup-completion-at-point ()
>   "Add `org-contacts-message-complete-function' as a new function
> to complete the thing at point."
>   (add-to-list 'completion-at-point-functions
> 	       'org-contacts-message-complete-function))

[ Side point: `add-to-list` is wrong, it should be `add-hook` (tho with
  additional "nil 'local" arguments, to only add to the buffer-local
  part of the hook).  ]

> That function basically checks if this is a place where completion
> should happen (To field, for instance), then calls all the
> org-contacts-complete-functions until one succeeds. One such function is
> org-contacts-complete-group that, in some case, returns a function
> taking unused arguments and returning the completion as a string. To
> summarize, is it okay for a function in completion-at-point-functions to
> return such a function?

The docstring of `completion-at-point-functions` says:

    Each function on this hook is called in turn without any argument and
    should return either nil, meaning it is not applicable at point,
    or a function of no arguments to perform completion (discouraged),
    or a list of the form (START END COLLECTION . PROPS), where:
    [...]

So, yes, it's acceptable (tho discouraged) to return a function, but this
function should then *perform the completion* (rather than just return
a string).

> I guess I should try to return something of the form (start end
> collection . props) instead, with collection being a singleton…

Why a singleton?  The driving design should be: return a `collection`
which is valid for any text between `start` and `end`.

Because `collection` will be matched against the text between `start`
and `end` anyway (and the UI may decide to match it against other text
as well, e.g. to provide other completion styles than just prefix
completion).


        Stefan