GNU bug report logs - #16555
24.3.50; Company and CAPF: dealing with completion values containing extra text

Previous Next

Package: emacs;

Reported by: Dmitry Gutov <dgutov <at> yandex.ru>

Date: Sun, 26 Jan 2014 04:12:02 UTC

Severity: normal

Found in version 24.3.50

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 16555 in the body.
You can then email your comments to 16555 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Sun, 26 Jan 2014 04:12:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Dmitry Gutov <dgutov <at> yandex.ru>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Sun, 26 Jan 2014 04:12:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Sun, 26 Jan 2014 06:11:23 +0200
When dealing with code completion for programming languages, it's usual
that candidates have associated text - most often, as function names,
they include the argument list.

Related issues:

* When inserting the "common part", we must ignore the extra text.

* The argument list is useful, for example when the candidate is
  inserted in the buffer. After it was explicitly selected, we also
  insert the arguments list, but replace each argument with self-erasing
  field when you type over it. We do that in company-clang and
  company-eclim (and could also do in company-senamtic, were someone to
  request that).

* C-like languages usually provide method overloading, and then the
  argument list is a part of a method's identity. Any metadata is
  associated with the tuple "method name" + arguments, so we want to
  pass the method name with arguments to functions that retrieve
  documentation, location, etc.

* If the completion table values only contained method names,
  `delete-duplicates' would remove all but one of the methods with the
  same name. But we do want to remove duplicates.

At the moment the following approach emerged (for examples, again, see
company-clang and company-eclim): the completion values include the
argument lists (but nothing extra except that), and the respective
backends define an undocumented command: `crop'.

That command is only used in two situations:

1. When `company-complete-common' is called, we insert the common part
among all candidates, but before that we call (backend-function 'crop
"common-part"), to make sure not to insert the paren or anything after
it.

2. When `company-auto-complete' feature is used, typing any character
from `company-auto-complete-chars' insert the currently selected
completion into the buffer. In that case we also only want to insert the
method name, and so backend's `crop' function is called.

When the candidate is inserted normally (by selecting it with M-p or M-n
and then pressing RET), `crop' is not called. The full candidate value
is inserted into the buffer, and then we call `post-completion' backend
command, to allow it to remove the arguments list, or do something more
advanced (see the * #2 above).

This behavior is inconsistent, and `crop' doesn't sounds too good as a
command name.

At the risk of breaking some existing code, and thanks to `crop' stil'
beging undocumented, I'd like to define a `value' command instead that
would return the "cleaned" candidate text, presumably without the
arguments list. It would get called anytime before the candidate text
gets inserted, and to get the above mentioned templatification behavior,
the `post-completion' code will need to explicitly insert the rest of
the candidate text (the full candidate will be passed as the first and
the only argument). Which will be the main difference from the backend's
writer perspective.

I see two problems:

* Including extra text in completion table seems like it won't mesh well
  with the completion-at-point-functions interface, and specifically
  with the completion-at-point as the frontend. How would one write a
  CAPF function for clang or eclim with argument lists in mind?

* `company-update-candidates' currently calls `company--safe-candidate'
  on the "common part", on the assumption that the `crop' command will
  return something meaningful for a string that's not itself a
  completion candidate (and currently, they all do, because they look
  for `(' instead of, say, using text properties or a hash table
  lookup). If the command is called `value', it would be more tempting
  for the implementor to only expect it to be called on actual
  candidates. And then it'll return nil or do something unexpected when
  called on something like "foo(".

Thoughts?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Sun, 26 Jan 2014 06:24:02 GMT) Full text and rfc822 format available.

Message #8 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Sun, 26 Jan 2014 01:23:15 -0500
> * Including extra text in completion table seems like it won't mesh well
>   with the completion-at-point-functions interface, and specifically
>   with the completion-at-point as the frontend. How would one write a
>   CAPF function for clang or eclim with argument lists in mind?

From a CAPF point of view, I think it would work as follows:
- all-completions would return the "cropped" names.
- `annotate-function' would be used to add the arglists to *Completions*.
- `exit-function' would be used to insert the arglist after selecting
  a candidate.


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Sun, 26 Jan 2014 22:53:01 GMT) Full text and rfc822 format available.

Message #11 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50; Company and CAPF: dealing with completion
 values containing extra text
Date: Mon, 27 Jan 2014 00:52:44 +0200
On 26.01.2014 08:23, Stefan Monnier wrote:
>> * Including extra text in completion table seems like it won't mesh well
>>    with the completion-at-point-functions interface, and specifically
>>    with the completion-at-point as the frontend. How would one write a
>>    CAPF function for clang or eclim with argument lists in mind?
>
>  From a CAPF point of view, I think it would work as follows:
> - all-completions would return the "cropped" names.
> - `annotate-function' would be used to add the arglists to *Completions*.

I played with this a bit, and looks like the proper way to make this 
work with the current completion-at-point code is to use text 
properties. I propertize the completions, and then the annotation 
function looks up the text property:

(add-hook 'completion-at-point-functions
           (lambda ()
             (let ((bounds (bounds-of-thing-at-point 'symbol)))
               (when bounds
                 (list
                  (car bounds)
                  (cdr bounds)
                  (list a1 a2 a3)
                  :annotation-function #'annota)))) nil t)

(setq a1 (propertize "aaa" 'a "a1")
      a2 (propertize "aaa" 'a "a2")
      a3 (propertize "aab" 'a "a3"))

(defun annota (s)
  (format "(%s)" (get-text-property 0 'a s)))

Using a hash table surprizingly didn't work, because:

* Without the additional text property, a1 and a2 are considered equal, 
and only one of them is listed in the completions buffer.
* If the hash table uses the `eq' test, lookup fails because the strings 
passed to the annotation function are not the same objects as those we 
return in the completions table.
* If the hash table uses the `equal' test, naturally it can't 
distinguish between a1 and a2 (lookup returns "a2" for both).

Is that how it's supposed to work?

And it looks to me that this approach is incompatible with the `value' 
command, if we want company-capf to support it. Using the annotation 
function, it would know how to get from value to value + annotation, but 
not the other way around.

Using `value' as described, aside from smoother transition from the 
current behavior, appealed to me because it allows to defer the string 
munging to the last possible moment, thus saving in performance on all 
strings that weren't displayed, when the candidates list is large.

But here's an extreme example, of sorts:

ELISP> (js2-time (all-completions "" obarray 'fboundp))
0.0121
ELISP> (js2-time (mapcar
           (lambda (s)
             (if (> (length s) 2)
                 (propertize s 's (substring s (/ (length s) 2)))
               s))
           (all-completions "" obarray 'fboundp)))
0.1318

The second measurement fluctuates between 130ms and 80ms, probably due 
to GC. Maybe this is negligible, considering that on my machine that's a 
collection with 19000 elements, and most completion lists will be 
considerably smaller.

On the other hand, using `annotate' cleanly separates the "meat" in 
completion candidates from the extra text, which can be used to e.g. 
visualize them differently, maybe with different faces and alignments in 
the popup. As long as we solve the issue of uniqueness.

> - `exit-function' would be used to insert the arglist after selecting
>    a candidate.

Yes, `exit-function' is the best match for `post-completion', but I 
don't see which value of STATUS should be considered as okay for 
insertion. `finished' seems to be a good candidate, but it does not seem 
to really correspond to when happens after `company-complete-selection' 
(the completion is inserted and the popup is closed). `finished' can 
only be the status when the inserted completion doesn't have any 
possible continuations in the completions table, whereas we obviously 
want to be able to do arglist insertion for these cases, too.

The reverse is also true: being able not to insert the arguments list 
for a sole candidate can also be useful, and in Company user can do that 
at least by repeatedly using TAB (company-complete-common) instead of 
`company-complete-selection'.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Mon, 27 Jan 2014 02:47:02 GMT) Full text and rfc822 format available.

Message #14 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Sun, 26 Jan 2014 21:46:34 -0500
> * Without the additional text property, a1 and a2 are considered equal, and
> only one of them is listed in the completions buffer.

Hmm??  AFAIK even with the text property, they are considered equal, so
it's odd that it would influence whether only one or both are displayed.
AFAIK in *Completions* "duplicates" are only eliminated if their
"text+annotation" is identical.

> And it looks to me that this approach is incompatible with the `value'
> command, if we want company-capf to support it.  Using the annotation
> function, it would know how to get from value to value + annotation,
> but not the other way around.

Agreed.

ELISP> (js2-time (all-completions "" obarray 'fboundp))
> 0.0121
ELISP> (js2-time (mapcar
>            (lambda (s)
>              (if (> (length s) 2)
>                  (propertize s 's (substring s (/ (length s) 2)))
>                s))
>            (all-completions "" obarray 'fboundp)))
> 0.1318

> The second measurement fluctuates between 130ms and 80ms, probably due to
> GC.  Maybe this is negligible, considering that on my machine
> that's a collection with 19000 elements, and most completion lists will be
> considerably smaller.

While I don't want to minimize the performance problem, you also have to
take into account the fact that the whole completion operation will
actually do something with those strings, so 19K candidates will
typically suffer from performance problems elsewhere.

> On the other hand, using `annotate' cleanly separates the "meat" in
> completion candidates from the extra text, which can be used to
> e.g. visualize them differently, maybe with different faces and alignments
> in the popup. As long as we solve the issue of uniqueness.

Conceptually, it does seem cleaner to me, indeed.

>> - `exit-function' would be used to insert the arglist after selecting
>> a candidate.
> Yes, `exit-function' is the best match for `post-completion', but I don't
> see which value of STATUS should be considered as okay for
> insertion.  `finished' seems to be a good candidate, but it does not seem to
> really correspond to when happens after `company-complete-selection' (the
> completion is inserted and the popup is closed).

It does correspond exactly.

> `finished' can only be the status when the inserted completion doesn't
> have any possible continuations in the completions table,

That description was from the point of view of "TAB-style completion".
In the case of company-complete-selection', we know that even if there
could be further continuations, the user's action indicates he doesn't
want those, so it really should be `finished'.

IOW the problem is one of how to better document the meaning of `finished'.

> The reverse is also true: being able not to insert the arguments list
> for a sole candidate can also be useful, and in Company user can do that at
> least by repeatedly using TAB (company-complete-common) instead of
> company-complete-selection'.

Then I guess that company-complete-common wouldn't want to pass
`finished' to the exit-function.


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Tue, 28 Jan 2014 05:38:02 GMT) Full text and rfc822 format available.

Message #17 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50; Company and CAPF: dealing with completion
 values containing extra text
Date: Tue, 28 Jan 2014 07:37:50 +0200
On 27.01.2014 04:46, Stefan Monnier wrote:
>> * Without the additional text property, a1 and a2 are considered equal, and
>> only one of them is listed in the completions buffer.
>
> Hmm??  AFAIK even with the text property, they are considered equal, so
> it's odd that it would influence whether only one or both are displayed.
> AFAIK in *Completions* "duplicates" are only eliminated if their
> "text+annotation" is identical.

Indeed, sorry, I made a wrong conclusion (when it tried it, the 
annotation function didn't work because it used the `eq' test).

But should we document that the only way to retain the information about 
different annotations corresponding to equal strings is to use text 
properties? I don't see any other way. Like mentioned, hash tables don't 
work:

(add-hook 'completion-at-point-functions
           (lambda ()
             (let ((bounds (bounds-of-thing-at-point 'symbol)))
               (when bounds
                 (list
                  (car bounds)
                  (cdr bounds)
                  (list a1 a2 a3)
                  :annotation-function #'annota)))) nil t)

(setq tt (make-hash-table :test 'eq)
      a1 "aaa"
      a2 "aaa"
      a3 "aab")

(puthash a1 "a1" tt)
(puthash a2 "a2" tt)
(puthash a3 "a3" tt)

(defun annota (s)
  (format "(%s)"
   (gethash s tt "?")))

aa| -> aaa(?), aab(?)

`company-capf' would be affected, too, since it uses 
`completion-all-completions' now, and the latter, via 
`completion-basic-all-completions', passes the completions through 
`completion-hilit-commonality', which intentionally creates new strings.

And if text properties is the only way, maybe disperse with the 
annotation function and just document the desired property on the strings?

> ELISP> (js2-time (all-completions "" obarray 'fboundp))
>> 0.0121
> ELISP> (js2-time (mapcar
>>             (lambda (s)
>>               (if (> (length s) 2)
>>                   (propertize s 's (substring s (/ (length s) 2)))
>>                 s))
>>             (all-completions "" obarray 'fboundp)))
>> 0.1318
>
>> The second measurement fluctuates between 130ms and 80ms, probably due to
>> GC.  Maybe this is negligible, considering that on my machine
>> that's a collection with 19000 elements, and most completion lists will be
>> considerably smaller.
>
> While I don't want to minimize the performance problem, you also have to
> take into account the fact that the whole completion operation will
> actually do something with those strings, so 19K candidates will
> typically suffer from performance problems elsewhere.

Well, I'm not sure: at least in this case, as you can see, fetching the 
candidates is an order or magnitude faster than separating their 
annotations. `all-completions' and `try-completion' are also faster than 
annotation processing, as well as `sort', but not, surprisingly, 
`delete-dups' (which can be replaced with `delete-consecutive-dups', 
however, which is also fast), and when displaying, we only need to work 
with a few elements of the whole completions list, so that takes more or 
less constant time.

But that's an extreme case: Emacs symbols, where we have the list 
readily at hand. As an opposite example, I have a completion backend 
which fetches candidates from an external Ruby process (also not the 
fastest of languages). Walking through all classes and returning all 
methods defined anywhere takes about 3.2s, the resulting list is 16K 
long in this case, and processing annotations takes 120ms. So in this 
case we're good.

>> `finished' can only be the status when the inserted completion doesn't
>> have any possible continuations in the completions table,
>
> That description was from the point of view of "TAB-style completion".
> In the case of company-complete-selection', we know that even if there
> could be further continuations, the user's action indicates he doesn't
> want those, so it really should be `finished'.
>
> IOW the problem is one of how to better document the meaning of `finished'.

But wouldn't it clash with the current completion-at-point interface? I 
don't think we can define a CAPF function without regard to how it work 
there.

>> The reverse is also true: being able not to insert the arguments list
>> for a sole candidate can also be useful, and in Company user can do that at
>> least by repeatedly using TAB (company-complete-common) instead of
>> company-complete-selection'.
>
> Then I guess that company-complete-common wouldn't want to pass
> `finished' to the exit-function.

It won't call the exit-function at all, currently, so when going though 
company-capf, exit-function would only be called with `finished'. That's 
another discrepancy, I guess.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Tue, 28 Jan 2014 13:25:02 GMT) Full text and rfc822 format available.

Message #20 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Tue, 28 Jan 2014 08:24:27 -0500
> But should we document that the only way to retain the information about
> different annotations corresponding to equal strings is to use text
> properties?

We could add a note somewhere, I guess.  So far, "different completions
for equal strings" is a very rare situation.

> And if text properties is the only way, maybe dispense with the annotation
> function and just document the desired property on the strings?

None of the annotation functions so far use text-properties, because the
completion candidate strings are all different anyway.  I'd rather not
force them to construct the annotation ahead of time "just in case it
might get displayed".

ELISP> (js2-time (mapcar
>>> (lambda (s)
>>> (if (> (length s) 2)
>>> (propertize s 's (substring s (/ (length s) 2)))
>>> s))
>>> (all-completions "" obarray 'fboundp)))

The comparison is not really fair.  Try comparing

   (mapcar (lambda (s)
             (if (> (length s) 2)
                 (substring s 0 (/ (length s) 2))
               s))
           (all-completions "" obarray 'fboundp)))
to
   (mapcar (lambda (s)
             (if (> (length s) 2)
                 (let ((bound (/ (length s) 2))
                       (comp (substring s 0 bound)))
                   (propertize comp 'annotation (substring s bound)))
               s))
           (all-completions "" obarray 'fboundp)))

which can be optimized to

   (mapcar (lambda (s)
             (if (> (length s) 2)
                 (let ((bound (/ (length s) 2))
                       (comp (substring s 0 bound)))
                   (put-text-property 0 bound 'annotation s comp)
                   comp)
               s))
           (all-completions "" obarray 'fboundp)))

Where the (substring s bound) is delayed to the annotation-function.

> Well, I'm not sure: at least in this case, as you can see, fetching the
> candidates is an order or magnitude faster than separating their
> annotations.

That's also because you artificially moved a lot of processing to the
all-completions step whereas some of that processing really belongs to
the annotation step.

>> That description was from the point of view of "TAB-style completion".
>> In the case of company-complete-selection', we know that even if there
>> could be further continuations, the user's action indicates he doesn't
>> want those, so it really should be `finished'.
>> IOW the problem is one of how to better document the meaning of `finished'.
> But wouldn't it clash with the current completion-at-point interface?

I don't see why.  The `finished' case is exactly for things like "insert
a terminator, record the user's choice somewhere, or some other thing
which only makes sense when you somehow know this part of the completion
is over".

> I don't think we can define a CAPF function without regard to how it
> work there.

Can't see why not.

>>> The reverse is also true: being able not to insert the arguments list
>>> for a sole candidate can also be useful, and in Company user can do that at
>>> least by repeatedly using TAB (company-complete-common) instead of
>>> company-complete-selection'.
>> Then I guess that company-complete-common wouldn't want to pass
>> `finished' to the exit-function.
> It won't call the exit-function at all, currently, so when going though
> company-capf, exit-function would only be called with `finished'.
> That's another discrepancy, I guess.

completion-at-point doesn't always call exit-function either.
Maybe Company could be improved to call the exit-function from
company-complete-common in the same way as completion-at-point, but
that's just a "quality of implementation" issue, it shouldn't affect
the API, AFAICT.


        Stefan




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Tue, 28 Jan 2014 16:01:02 GMT) Full text and rfc822 format available.

Message #23 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50; Company and CAPF: dealing with completion
 values containing extra text
Date: Tue, 28 Jan 2014 18:00:28 +0200
On 28.01.2014 15:24, Stefan Monnier wrote:
>> But should we document that the only way to retain the information about
>> different annotations corresponding to equal strings is to use text
>> properties?
>
> We could add a note somewhere, I guess.  So far, "different completions
> for equal strings" is a very rare situation.

Not "different completions", but rather different displayed information 
and exit behavior.

AFAICS, using annotation-function is a very rare situation by itself (in 
Emacs core, only `lisp-completion-at-point' does). I blame the 
bare-bones completion interface.

Note that we're discussing both CAPF and company-backends interfaces 
here. And "different completions for equal strings" would be true for 
company-eclim, company-clang, probably company-semantic eventually, and 
also third-party packages: emacs-eclim (it bundles a different Company 
backend), and omnisharp-emacs.

We could consider it a bug, though (that annotation-function is called 
with different objects). Currently, the fact that `-all-completions' 
functions for different styles process the returned list with 
`completion-hilit-commonality' in undocumented. Maybe we should push 
that logic somewhere, or just document that -all-completions functions 
must process annotations themselves first? Though sorting, in 
`minibuffer-completion-help', would become more complicated and slower 
as a result.

Come to think of it, company-backends should be able to use a hash-table 
with `eq' test maybe already, or if I massage the code a little. The 
major question for me was about uniqueness, and looks like, yes, doing 
`delete-consecutive-dups' after fetching all annotations should be fast 
enough (and this approach even has some space for optimization). So that 
leaves a problem with CAPF.

>> And if text properties is the only way, maybe dispense with the annotation
>> function and just document the desired property on the strings?
>
> None of the annotation functions so far use text-properties, because the
> completion candidate strings are all different anyway.

It could be an either/or specification: if annotation-function is 
defined, use it, otherwise, look up the `annotation' property.

And actually, `completion-all-completions' already conveys some 
information to the front-end using text properties: it uses `face' to 
delimit the "common" parts of the completions using 
`completion-hilit-commonality'.

> I'd rather not
> force them to construct the annotation ahead of time "just in case it
> might get displayed".

But completion-at-point calls annotation-function on all completions 
anyway: in `minibuffer-completion-help'. After sorting, but before 
deleting duplicates. Or otherwise, how would it only delete duplicates 
only where both value and annotation are the same?

>     (mapcar (lambda (s)
>               (if (> (length s) 2)
>                   (let ((bound (/ (length s) 2))
>                         (comp (substring s 0 bound)))
>                     (put-text-property 0 bound 'annotation s comp)
>                     comp)
>                 s))
>             (all-completions "" obarray 'fboundp)))

Using `put-text-property' is a good point.

> Where the (substring s bound) is delayed to the annotation-function.
>
...
> That's also because you artificially moved a lot of processing to the
> all-completions step whereas some of that processing really belongs to
> the annotation step.

But that step gets performed for all candidates anyway (see above).
Using `value-function' instead of `annotation-function' would have 
changed that. Although yes, that interface would be less neat.

>> But wouldn't it clash with the current completion-at-point interface?
>
> I don't see why.  The `finished' case is exactly for things like "insert
> a terminator, record the user's choice somewhere, or some other thing
> which only makes sense when you somehow know this part of the completion
> is over".
>
>> I don't think we can define a CAPF function without regard to how it
>> work there.
>
> Can't see why not.

Yeah, okay.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Tue, 28 Jan 2014 22:06:01 GMT) Full text and rfc822 format available.

Message #26 received at 16555 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 16555 <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Tue, 28 Jan 2014 17:04:39 -0500
>> We could add a note somewhere, I guess.  So far, "different completions
>> for equal strings" is a very rare situation.
> Not "different completions", but rather different displayed information and
> exit behavior.

Yes, that's what I meant by "different completions with equal strings".

> AFAICS, using annotation-function is a very rare situation by itself (in
> Emacs core, only `lisp-completion-at-point' does). I blame the bare-bones
> completion interface.

No need to blame anyone/anything.  CAPF needs to improve the annotation
support, indeed.  I don't think the current annotation-function is
sufficient, since there are different kinds of annotations.  E.g. adding
"<f>" is not the same as adding "(int x, float y, Vector<String>)" for
simple reasons of screen real estate, so in some UIs you'd want to
display both, while in others you'd only want the short one.

> We could consider it a bug, though (that annotation-function is called with
> different objects).

Not really: the "objects" you're talking about are returned by the
completion-table, i.e. they cover at most one "field" (in the
completion-boundaries sense), whereas completion-all-completions returns
strings that can span several fields, so clearly they can't always be
`eq' to something returned by the completion-table.

> Currently, the fact that `-all-completions' functions for different
> styles process the returned list with completion-hilit-commonality' in
> undocumented.

Indeed, but it's only part of the reason for the problem you see.
And there's no way to do the highlighting elsewhere: only the style
knows how to relate the input string (and point's position within in) to
the various output candidates.

> Maybe we should push that logic somewhere, or just document
> that -all-completions functions must process annotations
> themselves first?

The idea is rather to let the backend provide more kinds of annotations
and let the UI choose which one to use.  E.g. icomplete-mode doesn't
want any annotation at all, because its screen real-estate is
very limited.  So completion-all-completions can't blindly add annotations.

> Though sorting, in minibuffer-completion-help', would become more
> complicated and slower as a result.

I don't see how that's related.

> Come to think of it, company-backends should be able to use a hash-table
> with `eq' test maybe already, or if I massage the code a little. The major
> question for me was about uniqueness, and looks like, yes, doing
> delete-consecutive-dups' after fetching all annotations should be fast
> enough (and this approach even has some space for optimization). So that
> leaves a problem with CAPF.

That sounded like "thinking out loud for myself".  I don't know what you
wanted to say nor how that relates to CAPF.  FWIW,
minibuffer-completion-help uses `sort' on the "annotated completions"
and then display-completion-list uses delete-consecutive-dups (tho hand
written into the loop).

>>> And if text properties is the only way, maybe dispense with the annotation
>>> function and just document the desired property on the strings?
>> None of the annotation functions so far use text-properties, because the
>> completion candidate strings are all different anyway.
> It could be an either/or specification: if annotation-function is defined,
> use it, otherwise, look up the `annotation' property.

I think that in most cases the annotation function will want to do some
work rather than just return the content of the annotation (as in the
sample code in my previous message).

> But completion-at-point calls annotation-function on all completions anyway:

minibuffer-completion-help does.  completion-at-point also does when it
delegates to minibuffer-completion-help.  But minibuffer-force-complete
doesn't.  And neither does icomplete.

> After sorting, but before deleting duplicates.

Indeed.

> But that step gets performed for all candidates anyway (see above).

As shown above, no, not always.


        Stefan




Reply sent to Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility. (Tue, 28 Jan 2014 22:52:02 GMT) Full text and rfc822 format available.

Notification sent to Dmitry Gutov <dgutov <at> yandex.ru>:
bug acknowledged by developer. (Tue, 28 Jan 2014 22:52:02 GMT) Full text and rfc822 format available.

Message #31 received at 16555-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 16555-done <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50; Company and CAPF: dealing with completion
 values containing extra text
Date: Wed, 29 Jan 2014 00:51:17 +0200
On 29.01.2014 00:04, Stefan Monnier wrote:
> CAPF needs to improve the annotation
> support, indeed.  I don't think the current annotation-function is
> sufficient, since there are different kinds of annotations.  E.g. adding
> "<f>" is not the same as adding "(int x, float y, Vector<String>)" for
> simple reasons of screen real estate, so in some UIs you'd want to
> display both, while in others you'd only want the short one.

I'm not sure differentiating between them would be beneficial. We 
already have "full document" annotation (company-doc-buffer), "one-line" 
annotation (company-docsig), and just "annotation" itself. If we're 
going to differentiate between different kinds of short annotations, 
this will make 4 different functions a backend would need to define to 
describe a candidate with words.

FWIW, "(int x, float y, Vector<String>)" looks short enough to me. In 
Ruby, it often looks like "(table_name, column_name, [options])", which 
isn't too long and still allows completion-at-point display candidates 
in two columns.

>> We could consider it a bug, though (that annotation-function is called with
>> different objects).
>
> Not really: the "objects" you're talking about are returned by the
> completion-table, i.e. they cover at most one "field" (in the
> completion-boundaries sense), whereas completion-all-completions returns
> strings that can span several fields, so clearly they can't always be
> `eq' to something returned by the completion-table.

I see. Then I'm out of ideas here, and using text properties, as 
non-obvious that is, indeed remains the best option.

> The idea is rather to let the backend provide more kinds of annotations
> and let the UI choose which one to use.  E.g. icomplete-mode doesn't
> want any annotation at all, because its screen real-estate is
> very limited.  So completion-all-completions can't blindly add annotations.

Thanks for pointing that out.

>> Come to think of it, company-backends should be able to use a hash-table
>> with `eq' test maybe already, or if I massage the code a little. The major
>> question for me was about uniqueness, and looks like, yes, doing
>> delete-consecutive-dups' after fetching all annotations should be fast
>> enough (and this approach even has some space for optimization). So that
>> leaves a problem with CAPF.
>
> That sounded like "thinking out loud for myself".  I don't know what you
> wanted to say nor how that relates to CAPF.

It was. Sorry if it's out of place.
I filed this bug for discussing a new feature in both Company and CAPF, 
and that was me summing up the (one-sided) discussion of it on the 
Company side. So, closing.

> FWIW,
> minibuffer-completion-help uses `sort' on the "annotated completions"
> and then display-completion-list uses delete-consecutive-dups (tho hand
> written into the loop).

Yes, delete-consecutive-dups in Company is also inlined (in 
`company-calculate-candidates'), we couldn't use the function itself 
anyway, cause it's 24.4-only.

>> It could be an either/or specification: if annotation-function is defined,
>> use it, otherwise, look up the `annotation' property.
>
> I think that in most cases the annotation function will want to do some
> work rather than just return the content of the annotation (as in the
> sample code in my previous message).

Hm, yes, indeed.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#16555; Package emacs. (Wed, 29 Jan 2014 01:35:01 GMT) Full text and rfc822 format available.

Message #34 received at 16555-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 16555-done <at> debbugs.gnu.org
Subject: Re: bug#16555: 24.3.50;
 Company and CAPF: dealing with completion values containing extra text
Date: Tue, 28 Jan 2014 20:34:34 -0500
>> CAPF needs to improve the annotation support, indeed.  I don't think
>> the current annotation-function is sufficient, since there are
>> different kinds of annotations.  E.g. adding "<f>" is not the same as
>> adding "(int x, float y, Vector<String>)" for simple reasons of
>> screen real estate, so in some UIs you'd want to display both, while
>> in others you'd only want the short one.
> I'm not sure differentiating between them would be beneficial. We already
> have "full document" annotation (company-doc-buffer), "one-line" annotation
> (company-docsig), and just "annotation" itself.  If we're going to
> differentiate between different kinds of short annotations, this will make
> 4 different functions a backend would need to define to describe a candidate
> with words.
> FWIW, "(int x, float y, Vector<String>)" looks short enough to me.  In Ruby,
> it often looks like "(table_name, column_name, [options])", which isn't too
> long and still allows completion-at-point display candidates in two columns.

Hmm... I guess you're right.

> I see.  Then I'm out of ideas here, and using text properties, as
> non-obvious that is, indeed remains the best option.

Agreed.

>> That sounded like "thinking out loud for myself".  I don't know what you
>> wanted to say nor how that relates to CAPF.
> It was. Sorry if it's out of place.

No, no, feel free to think out loud.  I just wasn't sure if I'd missed
something or what.

> I filed this bug for discussing a new feature in both Company and
> CAPF, and that was me summing up the (one-sided) discussion of it on
> the Company side.  So, closing.

So, IIUC the conclusion is "if the string's chars is not enough, you
have to store the extra info in text-properties".  Not sure where we
could put this info, but if you can think of a place, feel free to add it.


        Stefan




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 26 Feb 2014 12:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 10 years and 63 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.