bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

1. Open an IELM buffer and (on Unix) type `"/' there.

2. Leave the point after `/'.

3. Type `M-x company-capf', see that all candidates have the first
character missing.

This is caused by `(comint--match-partial-filename)' matching the full
file path, including the leading slash, but
`comint-completion-file-name-table' returning base names, without any
path separators. Not sure how `completion-at-point' ignores that
problem.

Bonus round:

4. Type `/u', see the suggestion to complete it to `/ur/'.

5. Type `/usr/', then `M-x company-capf', see an error caused by some
candidates being shorter than the prefix.

In GNU Emacs 24.3.50.4 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.6)
 of 2014-01-04 on axl
Bzr revision: 115859 vincentb1@users.sourceforge.net-20140103141824-juu968y9pi6r2zvv
Windowing system distributor `The X.Org Foundation', version 11.0.11403000
System Description:	Ubuntu 13.10

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

> 1. Open an IELM buffer and (on Unix) type `"/' there.
> 2. Leave the point after `/'.
> 3. Type `M-x company-capf', see that all candidates have the first
> character missing.

That's normal.  Try C-x C-f / TAB TAB and you'll see that the leading /
is also "missing" in the *Completions* buffer.

> Not sure how `completion-at-point' ignores that problem.

It doesn't ignore the problem.  It knows that (all-completions STR
TABLE) doesn't always return strings that have STR as a prefix and
spends a fair bit of effort handling it right.

> 5. Type `/usr/', then `M-x company-capf', see an error caused by some
> candidates being shorter than the prefix.

Indeed, Company can't handle all completion-at-point-functions so far
because it assumes all completion tables are "simple", unlike
for example filename completion.

You can know how many chars are "missing" by calling
`completion-boundaries'.  Ideally, Company should be extended to handle
this feature, but for now that can be handled in company-capf.


        Stefan

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

On 04.01.2014 09:00, Stefan Monnier wrote:
> That's normal.  Try C-x C-f / TAB TAB and you'll see that the leading /
> is also "missing" in the *Completions* buffer.
 > ...
 > Indeed, Company can't handle all completion-at-point-functions so far
 > because it assumes all completion tables are "simple", unlike
 > for example filename completion.
 >
 > You can know how many chars are "missing" by calling
 > `completion-boundaries'.

Thanks, I didn't know about that.

 > Ideally, Company should be extended to handle this feature,

It's not hard to do, but are you sure it would be a good addition to the 
API? "Completion prefix" and "completion bounds" are easy to mix up, and 
from what I see in various completion mechanisms, the non-simple 
completion tables more often need to look at the whole buffer before 
point, or at least a large chunk of it.

`completion-file-name-table' is more of an exception, I think. But if it 
was only passed the segment of STRING after the last path separator, it 
could still look behind it in the buffer and see the full path.

 > but for now that can be handled in company-capf.

Ok, I'll try.

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

On 05.01.2014 06:20, Dmitry Gutov wrote:
> Ok, I'll try.

Fixed upstream.

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

> `completion-file-name-table' is more of an exception, I think.  But if
> it was only passed the segment of STRING after the last path
> separator, it could still look behind it in the buffer and see the
> full path.

But the completion may actually want to *change* the text before
the boundary.  E.g. completion of /u/s/d to /usr/share/doc.

The completion table itself does not know how to do that, but some
completion styles do (e.g. partial-completion).
So the completion-at-point-function needs to indicate the boundaries of
"/u/s/d" which indicate which text can be affected by the completion,
while `completion-boundaries' tells the lower-level completion code
(e.g. the one in partial-completion) how to split the completion field
into sub-fields.


        Stefan

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

On 05.01.2014 08:53, Stefan Monnier wrote:
>> `completion-file-name-table' is more of an exception, I think.  But if
>> it was only passed the segment of STRING after the last path
>> separator, it could still look behind it in the buffer and see the
>> full path.
>
> But the completion may actually want to *change* the text before
> the boundary.  E.g. completion of /u/s/d to /usr/share/doc.

In that case, "/usr/share/doc" is the completion candidate, not "doc", 
right? Then, "/u/s/d" can be considered a "generalized" prefix, or the 
entity to complete, in c-a-p-f terms. And during completion we delete 
the latter and insert the former.

> The completion table itself does not know how to do that, but some
> completion styles do (e.g. partial-completion).
> So the completion-at-point-function needs to indicate the boundaries of
> "/u/s/d" which indicate which text can be affected by the completion,
> while `completion-boundaries' tells the lower-level completion code
> (e.g. the one in partial-completion) how to split the completion field
> into sub-fields.

To be clear, I'm not convinced that the notion of "sub-fields" is 
useful. Defining limits to the text that can be affected by completion 
only looks good to me from the presentation point of view: if the 
candidate strings can be shorter, we can show more of them in the 
*Candidates* buffer, whereas it's less useful for popup-style UIs where 
the candidates are displayed vertically anyway.

IOW, if I were to add a `boundaries' action to company-backends API, it 
would only be used for presentation: the popup will cut off that many 
characters from the candidate strings, and it will be rendered that many 
columns to the right.

And maybe do the same for suffix boundary, when/if we support that kind 
of completion.

Come to think of it, though, this new action may be incompatible with 
the notion of merged backends. If we have candidates that come from 
backends that return the same prefix but different boundaries, there's 
no way to reflect the boundaries in the popup.

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

>>> `completion-file-name-table' is more of an exception, I think.  But if
>>> it was only passed the segment of STRING after the last path
>>> separator, it could still look behind it in the buffer and see the
>>> full path.
>> But the completion may actually want to *change* the text before
>> the boundary.  E.g. completion of /u/s/d to /usr/share/doc.
> In that case, "/usr/share/doc" is the completion candidate, not "doc",
> right?

Not sure what you mean by "completion candidate":

try-completion and all-completions will both return nil because there's
no "/u/s" directory.  Assuming we use partial-completion style,
completion-try-completion should return "/usr/share/doc" and
completion-all-completions should return ("usr/share/doc"), i.e. without
the leading "/".  If we had started from "/usr/s/d" the results would
have been the same except completion-all-completions would return
("share/doc").

> To be clear, I'm not convinced that the notion of "sub-fields" is
> useful. Defining limits to the text that can be affected by completion only
> looks good to me from the presentation point of view: if the candidate
> strings can be shorter, we can show more of them in the *Candidates* buffer,
> whereas it's less useful for popup-style UIs where the candidates are
> displayed vertically anyway.

Then just have company-capf check completion-boundaries and concat the
missing prefix to every element returned by all-completions.

> IOW, if I were to add a `boundaries' action to company-backends API, it
> would only be used for presentation: the popup will cut off that many
> characters from the candidate strings, and it will be rendered that many
> columns to the right.

If you want to let Company provide completion styles like
partial-completion you'll need some additional info about "subfields".
But as long as you limit yourself to prefix or substring completion you
don't need that.

> Come to think of it, though, this new action may be incompatible with the
> notion of merged backends. If we have candidates that come from backends
> that return the same prefix but different boundaries, there's no way to
> reflect the boundaries in the popup.

Yup.  Just like you have a problem when the start/end of the
completion text is not identical.  E.g. you could have a "word" backend
and a "varname" backend, and you type "my_fanc" and now the "word"
backend wants to complete "fanc" whereas the varname backend wants to
complete "my_fanc".


        Stefan

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

On 06.01.2014 19:23, Stefan Monnier wrote:
>>>> `completion-file-name-table' is more of an exception, I think.  But if
>>>> it was only passed the segment of STRING after the last path
>>>> separator, it could still look behind it in the buffer and see the
>>>> full path.
>>> But the completion may actually want to *change* the text before
>>> the boundary.  E.g. completion of /u/s/d to /usr/share/doc.

Maybe I didn't ask the right question. If the completion may want to 
change the text outside of its defined boundaries, doesn't that subvert 
the meaning of the "boundaries"? Or do you mean that, depending on the 
completion style, `completion-file-name-table' may return different 
boundaries, with the same prefix and suffix? So that modifications will 
still remain within the boundaries?

>> In that case, "/usr/share/doc" is the completion candidate, not "doc",
>> right?
>
> Not sure what you mean by "completion candidate":

Elements returned by the completion table?
Strings of text, one of which is likely to replace the text between 
completion boundaries.

> try-completion and all-completions will both return nil because there's
> no "/u/s" directory.  Assuming we use partial-completion style,
> completion-try-completion should return "/usr/share/doc" and
> completion-all-completions should return ("usr/share/doc"), i.e. without
> the leading "/".  If we had started from "/usr/s/d" the results would
> have been the same except completion-all-completions would return
> ("share/doc").

Sounds quite convoluted.

>> To be clear, I'm not convinced that the notion of "sub-fields" is
>> useful. Defining limits to the text that can be affected by completion only
>> looks good to me from the presentation point of view: if the candidate
>> strings can be shorter, we can show more of them in the *Candidates* buffer,
>> whereas it's less useful for popup-style UIs where the candidates are
>> displayed vertically anyway.
>
> Then just have company-capf check completion-boundaries and concat the
> missing prefix to every element returned by all-completions.

Already done: 
https://github.com/company-mode/company-mode/compare/b70540b5fcd062c4670dea7004453de326ff4f70...8ecec3594931ae8e2329fec4b793ad4ba392e4ef

>> IOW, if I were to add a `boundaries' action to company-backends API, it
>> would only be used for presentation: the popup will cut off that many
>> characters from the candidate strings, and it will be rendered that many
>> columns to the right.
>
> If you want to let Company provide completion styles like
> partial-completion you'll need some additional info about "subfields".
> But as long as you limit yourself to prefix or substring completion you
> don't need that.

I wonder if this information has to be reflected in the backend API. For 
example, some external service might want to pick the completion styles 
itself, and we can just give the backend prefix, get back a list of 
completion candidates, and eventually replace the prefix with one of the 
candidates. Same with suffix whenever we get around to supporting that.

 From where I stand, knowing the completion style in advance only gives 
us benefits in visualization (know which characters in candidate strings 
match) and, in some cases, performance (no need to do concatenation even 
if the strings we receive from some subsystem have some offset in the 
prefix).

On the other hand, the backend is free to try all completion styles it 
knows/is allowed to use, sort and return together, like in this feature 
request: 
https://github.com/company-mode/company-mode/issues/45#issuecomment-31564029, 
saving several network road-trips and possibly further amount of CPU time.

And as far as highlighting the matching characters in candidates goes, 
we could just skip that for non-prefix matches (checking each on the 
fly), or do it sloppily, without regard for the backend logic.

>> Come to think of it, though, this new action may be incompatible with the
>> notion of merged backends. If we have candidates that come from backends
>> that return the same prefix but different boundaries, there's no way to
>> reflect the boundaries in the popup.
>
> Yup.  Just like you have a problem when the start/end of the
> completion text is not identical.  E.g. you could have a "word" backend
> and a "varname" backend, and you type "my_fanc" and now the "word"
> backend wants to complete "fanc" whereas the varname backend wants to
> complete "my_fanc".

We currently deal with that by only using the backends that respond with 
the same prefix as the first one that returned non-nil. Guess we could 
add the boundaries to the comparison list, too...

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

>>>>> `completion-file-name-table' is more of an exception, I think.  But if
>>>>> it was only passed the segment of STRING after the last path
>>>>> separator, it could still look behind it in the buffer and see the
>>>>> full path.
>>>> But the completion may actually want to *change* the text before
>>>> the boundary.  E.g. completion of /u/s/d to /usr/share/doc.
> Maybe I didn't ask the right question. If the completion may want to change
> the text outside of its defined boundaries, doesn't that subvert the meaning
> of the "boundaries"?

There are 2 levels: the level of try-completion, all-completions, an
completion-boundaries.  At that level, the completion should not affect
the text outside of the boundaries.  This is the level at which
completion-tables operate.

Then there's the level of completion-try-completion and
completion-all-completions.  This uses the info from the previous level
to implement completion according to completion-styles.  It can change
things outside of the boundaries.  This is part of the UI, not of
the backends.

>>> In that case, "/usr/share/doc" is the completion candidate, not "doc",
>>> right?
>> Not sure what you mean by "completion candidate":
> Elements returned by the completion table?

Then "/u/s/d" returns no completion candidates.

> Already done:
> https://github.com/company-mode/company-mode/compare/b70540b5fcd062c4670dea7004453de326ff4f70...8ecec3594931ae8e2329fec4b793ad4ba392e4ef

It's not in elpa yet.

> On the other hand, the backend is free to try all completion styles it

The backends know nothing about completion styles.

> We currently deal with that by only using the backends that respond with the
> same prefix as the first one that returned non-nil. Guess we could add the
> boundaries to the comparison list, too...

Exactly.


        Stefan

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

On 08.01.2014 07:33, Stefan Monnier wrote:
>> Already done:
>> https://github.com/company-mode/company-mode/compare/b70540b5fcd062c4670dea7004453de326ff4f70...8ecec3594931ae8e2329fec4b793ad4ba392e4ef
>
> It's not in elpa yet.

It will be. Users won't get the new version until the header's bumped 
anyway.

>> On the other hand, the backend is free to try all completion styles it
>
> The backends know nothing about completion styles.

Yes, but is this the best approach? I see you're taking advantage of 
`completion-regexp-list' and the fact that `all-completions' is 
implemented in C in `completion-pcm--all-completions', but if one would 
implement a completion function using an external service, in many cases 
this would mean a non-optimal amount of data to have to be transferred.

And a service's implementation of different completion styles could be 
just as fast, if not faster. Omnisharp has it already, so using it 
should make sense.

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

>>> On the other hand, the backend is free to try all completion styles it
>> The backends know nothing about completion styles.
> Yes, but is this the best approach? I see you're taking advantage of
> completion-regexp-list' and the fact that `all-completions' is implemented
> in C in `completion-pcm--all-completions', but if one would
> implement a completion function using an external service, in many cases
> this would mean a non-optimal amount of data to have to be transferred.

That's true.  That function could make use of completion-regexp-list,
but maybe it can't support full regexps (and even if it can, it then
requires translating Emacs regexps to the format used by the external
tool).

This is a limitation inherited from the "original" completion
infrastructure.  We could easily introduce a replacement for
completion-regexp-list which specifies a "pattern" (in a format to be
defined).

This said, even if the external service can't just take an Emacs regexp,
the completion function can parse the regexp from completion-regexp-list
and turn it into a different/simpler pattern.

> And a service's implementation of different completion styles could be just
> as fast, if not faster.

Of course, it's only interesting if it's faster, otherwise, why bother?


        Stefan

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Dmitry Gutov]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> This is a limitation inherited from the "original" completion
> infrastructure.  We could easily introduce a replacement for
> completion-regexp-list which specifies a "pattern" (in a format to be
> defined).

A list of segments would a natural choice, but I was thinking about a
more radical change: allowing backends to handle the completion styles.
At the moment, `completion--nth-completion' tries all allowed styles,
and returns the candidates for the first style that succeeds.

Firstly, one might prefer to see candidates for several styles, ranked
and merged, like described here:
https://github.com/company-mode/company-mode/issues/45#issuecomment-31564029

We could tell the backends about accepted styles (maybe it would be in
their metadata if they can handle styles on their own), and then trust
them to return appropriate results.

The main problem with this approach would be to make sure frontends can
handle not knowing which style was used to find a given completion
candidate. Or maybe require backends to hand over that information.

>> And a service's implementation of different completion styles could be just
>> as fast, if not faster.
>
> Of course, it's only interesting if it's faster, otherwise, why bother?

Even if the filtering speed is about the same, speedup can come from
less time spent serializing/deserializing the full candidates list.

And, if we want to return results from different styles together, from
making less network calls, and from not forcing the backend to do any
preprocessing required by completion code multiple times.

bug#16334: 24.3.50; company-capf eats the first char in IELM filename completions

[Stefan Monnier]

> A list of segments would a natural choice, but I was thinking about a
> more radical change: allowing backends to handle the completion styles.

I guess the backend could provide a property which would cause
completion--nth-completion to just pass the request to the completion-table.

That would be an easy change in minibuffer.el.  But I'd first like to
see how that would work out on the backend side: it might turn out to be
pretty difficult to make it work well, or at least it might require
exposing more of minibuffer.el's code (i.e. turn some "--" into "-",
which implies thinking hard about whether that's really an API with
which we want to live for a long time).

Unless it is used to override the completion-styles: the omnisharp
completion could simply ignore completion-styles and use its own
omnisharp-server-provided behavior.

> Firstly, one might prefer to see candidates for several styles, ranked
> and merged, like described here:
> https://github.com/company-mode/company-mode/issues/45#issuecomment-31564029

That sounds like a request that applies to all backends, i.e. is only
part of the UI.

> The main problem with this approach would be to make sure frontends can
> handle not knowing which style was used to find a given completion
> candidate.

Currently the only UI which uses completion-styles indeed has no idea
which style was used and that hasn't been a problem.  In general,
I can't see why that info would be necessary.


        Stefan