Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Dmitry Gutov]

Hi Stefan,

> ------------------------------------------------------------
> revno: 395
> committer: Stefan Monnier <monnier-CRDzTM1onBSWkKpYnGOUKg@public.gmane.org>
> branch nick: elpa
> timestamp: Sat 2013-04-27 09:48:47 -0400
> message:

>  ;; (defun company-my-backend (command &optional arg &rest ignored)
> -;;   (case command
> -;;     (prefix (when (looking-back "foo\\>")
> +;;   (pcase command
> +;;     (`prefix (when (looking-back "foo\\>")
>  ;;               (match-string 0)))
> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
> -;;     (meta (format "This value is named %s" arg))))
> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
> +;;     (`meta (format "This value is named %s" arg))))

Like the header in company.el says, we still try to support Emacs 22 and
23. `pcase' was only added in 23.3, I believe.

> +(defun company-capf (command &optional arg &rest _args)
...
> +    (duplicates nil) ;Don't bother.
> +    (no-cache t)     ;FIXME: Improve!
> +    (meta nil)       ;FIXME: Return one-line docstring for `arg'.
> +    (doc-buffer nil) ;FIXME: Return help buffer for `arg'.
> +    (location nil)   ;FIXME: Return (BUF . POS) or (FILE . LINENB) of
> `arg'.
> +    (init nil)      ;Don't bother: plenty of other ways to initialize
> the code.

a) There's no need to return nils explicitly, other backends don't.

b) Who are these FIXMEs for? I guess `meta' can be implemented by
calling `:annotation-function' (don't know if it's appropriate), but
`doc-buffer' and `location' don't have anything corresponding in
`completion-extra-properties'.

> +    (require-match nil)            ;This should be a property of the
> front-end!

Should it really? A smart back-end could determine if we're typing a
function call, or typing the name for a new function, and at least
override it to `never' in the latter case.

Returning `t' might indeed never be appropriate for a back-end, and some
front-ends could want to have a say in this, but I'd rather see them in
practice first.

> +(defvar company-backend)
> +

This variable is declared about ~30 lines below that. Is it appropriate
to have both declarations in the same file?

>  Example:
>  \(company-begin-with '\(\"foo\" \"foobar\" \"foobarbaz\"\)\)"
> +  ;; FIXME: Shouldn't `company-begin-with-marker' be removed and replaced
> +  ;; by a lexical variable?

It probably should, but, again, lexical-binding is not available in
Emacs < 24. ATM I'm only using it to improve some logic that uses
`boundp' in `company-elisp'.

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Stefan Monnier]

>> ;; (defun company-my-backend (command &optional arg &rest ignored)
>> -;;   (case command
>> -;;     (prefix (when (looking-back "foo\\>")
>> +;;   (pcase command
>> +;;     (`prefix (when (looking-back "foo\\>")
>> ;;               (match-string 0)))
>> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
>> -;;     (meta (format "This value is named %s" arg))))
>> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
>> +;;     (`meta (format "This value is named %s" arg))))

> Like the header in company.el says, we still try to support Emacs 22 and
> 23. `pcase' was only added in 23.3, I believe.

But this is in a comment, so it's not a problem.

>> +(defun company-capf (command &optional arg &rest _args)
> ...
>> +    (duplicates nil) ;Don't bother.
>> +    (no-cache t)     ;FIXME: Improve!
>> +    (meta nil)       ;FIXME: Return one-line docstring for `arg'.
>> +    (doc-buffer nil) ;FIXME: Return help buffer for `arg'.
>> +    (location nil)   ;FIXME: Return (BUF . POS) or (FILE . LINENB) of `arg'.
>> +    (init nil)      ;Don't bother: plenty of other ways to initialize
>> the code.
> a) There's no need to return nils explicitly, other backends don't.

Indeed, I only need them to have a place where to put the FIXMEs.

> b) Who are these FIXMEs for?

Whoever takes on the challenge.

> I guess `meta' can be implemented by
> calling `:annotation-function' (don't know if it's appropriate), but
> `doc-buffer' and `location' don't have anything corresponding in
> `completion-extra-properties'.

`completion-extra-properties' can have anything you want in it.
Same for `completion-metadata'.  Of course, new entries in these
alists/plists won't be understood by other users of
completion-at-point-functions, but that's not a problem in itself.

>> +    (require-match nil)            ;This should be a property of the
>> front-end!
> Should it really?

Hmm... after writing an answer and throwing it away a few times, I think
you're right: this definitely makes sense for a backend and basically
means "I, as a backend, know that only those elements make sense here".

But in most cases the front-end can't use it to *prevent* the user from
writing something else (Emacs likes to let the user shoot himself in the
foot, on the assumption that the user knows better).  It could use it to
highlight a non-matching entry, OTOH.

>> +(defvar company-backend)
> This variable is declared about ~30 lines below that. Is it appropriate
> to have both declarations in the same file?

No, it was a quick fix.  Moving the declaration is probably a better option.

> It probably should, but, again, lexical-binding is not available in
> Emacs < 24. ATM I'm only using it to improve some logic that uses
> `boundp' in `company-elisp'.

I work under the assumption that my time is better spent if
I concentrate on users of Emacs-24 (users who want the newer features of
company can upgrade to Emacs-24).


        Stefan

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Dmitry Gutov]

On 05.05.2013 10:32, Stefan Monnier wrote:
>>> ;; (defun company-my-backend (command &optional arg &rest ignored)
>>> -;;   (case command
>>> -;;     (prefix (when (looking-back "foo\\>")
>>> +;;   (pcase command
>>> +;;     (`prefix (when (looking-back "foo\\>")
>>> ;;               (match-string 0)))
>>> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
>>> -;;     (meta (format "This value is named %s" arg))))
>>> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
>>> +;;     (`meta (format "This value is named %s" arg))))
>
>> Like the header in company.el says, we still try to support Emacs 22 and
>> 23. `pcase' was only added in 23.3, I believe.
>
> But this is in a comment, so it's not a problem.

I'm not sure recommending to users to write new backends in a way 
incompatible with older Emacs is good. They may wish to remain 
compatible, yet be unaware that `pcase' is a new feature.
That might make them sad upon discovery, and doubly so if they don't 
know about cl's `case'.

By the way, is there a reason to use backquotes here? I remember you 
removing quotes from all backends' `case' forms, and using unquoted 
symbols in `pcase' seems to work just as well:

(pcase 'boo (boo 'bah)) => 'bah

>>> +(defun company-capf (command &optional arg &rest _args)
>> ...
>>> +    (duplicates nil) ;Don't bother.
>>> +    (no-cache t)     ;FIXME: Improve!
>>> +    (meta nil)       ;FIXME: Return one-line docstring for `arg'.
>>> +    (doc-buffer nil) ;FIXME: Return help buffer for `arg'.
>>> +    (location nil)   ;FIXME: Return (BUF . POS) or (FILE . LINENB) of `arg'.
>>> +    (init nil)      ;Don't bother: plenty of other ways to initialize
>>> the code.
>> a) There's no need to return nils explicitly, other backends don't.
>
> Indeed, I only need them to have a place where to put the FIXMEs.

I guess I'll remove `(init nil)', then.
By the way, if you think that `init' is useless for all backends, I 
don't believe that's true for `ropemacs' and `clang'. Retrying 
initialization once per buffer is fairly useful.

>> b) Who are these FIXMEs for?
>
> Whoever takes on the challenge.
>
>> I guess `meta' can be implemented by
>> calling `:annotation-function' (don't know if it's appropriate), but
>> `doc-buffer' and `location' don't have anything corresponding in
>> `completion-extra-properties'.
>
> `completion-extra-properties' can have anything you want in it.
> Same for `completion-metadata'.  Of course, new entries in these
> alists/plists won't be understood by other users of
> completion-at-point-functions, but that's not a problem in itself.

Ah okay, so I guess that section has everything to do with moving some 
company backends to completion-at-point functions.

>>> +    (require-match nil)            ;This should be a property of the
>>> front-end!
>> Should it really?
>
> Hmm... after writing an answer and throwing it away a few times, I think
> you're right: this definitely makes sense for a backend and basically
> means "I, as a backend, know that only those elements make sense here".
>
> But in most cases the front-end can't use it to *prevent* the user from
> writing something else (Emacs likes to let the user shoot himself in the
> foot, on the assumption that the user knows better).  It could use it to
> highlight a non-matching entry, OTOH.

Yes, so different completion UIs can interpret that return value 
differently. `completion-at-point' will ignore it.

`company' front-ends can't highlight the non-matching input, though, 
because it effectively restarts completion after each character the user 
inputs (with some caching, to speed up the process), and so non-matching 
input aborts the completion and hides the candidates. This is somewhat 
difficult to change.

But anyway, it follows the model set by IDEs which usually require 
matching input during completion, because the completion dropdown is 
often used for API discovery by the programmer, and it's often not big 
enough to show all candidates at the same time.

The foot-shooting restriction is alleviated by the fact the the user can 
disable it (unless the backend returned `t', but, to my knowledge, none 
of them do that, and I might eventually remove that possibility), and 
that it only comes into play after the user has interacted with the 
completion interface, i.e. not when completion was just initiated by the 
timer.

>> It probably should, but, again, lexical-binding is not available in
>> Emacs < 24. ATM I'm only using it to improve some logic that uses
>> `boundp' in `company-elisp'.
>
> I work under the assumption that my time is better spent if
> I concentrate on users of Emacs-24 (users who want the newer features of
> company can upgrade to Emacs-24).

This is totally fine for `company-capf', but `company-begin-with' is an 
old function/feature. I wonder if anyone's actually using it, though.

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Sebastian Wiesner]

2013/5/5 Dmitry Gutov <dgutov@yandex.ru>:
> On 05.05.2013 10:32, Stefan Monnier wrote:
>>>>
>>>> ;; (defun company-my-backend (command &optional arg &rest ignored)
>>>> -;;   (case command
>>>> -;;     (prefix (when (looking-back "foo\\>")
>>>> +;;   (pcase command
>>>> +;;     (`prefix (when (looking-back "foo\\>")
>>>> ;;               (match-string 0)))
>>>> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
>>>> -;;     (meta (format "This value is named %s" arg))))
>>>> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
>>>> +;;     (`meta (format "This value is named %s" arg))))
>>
> […]
>
> By the way, is there a reason to use backquotes here? I remember you
> removing quotes from all backends' `case' forms, and using unquoted symbols
> in `pcase' seems to work just as well:
>
> (pcase 'boo (boo 'bah)) => 'bah

This clause does **not** match the symbol 'boo.  It matches
**anything** and binds it to "boo". See

(pcase 'boo (foo foo) (boo 'bah)) => 'boo

compared to:

(pcase 'boo (`foo foo) (`boo 'bah)) => 'bah

So the backquote is actually mandatory here.

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Dmitry Gutov]

On 05.05.2013 14:05, Sebastian Wiesner wrote:
> 2013/5/5 Dmitry Gutov <dgutov@yandex.ru>:
>> On 05.05.2013 10:32, Stefan Monnier wrote:
>>>>>
>>>>> ;; (defun company-my-backend (command &optional arg &rest ignored)
>>>>> -;;   (case command
>>>>> -;;     (prefix (when (looking-back "foo\\>")
>>>>> +;;   (pcase command
>>>>> +;;     (`prefix (when (looking-back "foo\\>")
>>>>> ;;               (match-string 0)))
>>>>> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
>>>>> -;;     (meta (format "This value is named %s" arg))))
>>>>> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
>>>>> +;;     (`meta (format "This value is named %s" arg))))
>>>
>> […]
>>
>> By the way, is there a reason to use backquotes here? I remember you
>> removing quotes from all backends' `case' forms, and using unquoted symbols
>> in `pcase' seems to work just as well:
>>
>> (pcase 'boo (boo 'bah)) => 'bah
>
> This clause does **not** match the symbol 'boo.  It matches
> **anything** and binds it to "boo". See
>
> (pcase 'boo (foo foo) (boo 'bah)) => 'boo
>
> compared to:
>
> (pcase 'boo (`foo foo) (`boo 'bah)) => 'bah
>
> So the backquote is actually mandatory here.

Oh, I see. Thanks!

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Stefan Monnier]

>>>> ;; (defun company-my-backend (command &optional arg &rest ignored)
>>>> -;;   (case command
>>>> -;;     (prefix (when (looking-back "foo\\>")
>>>> +;;   (pcase command
>>>> +;;     (`prefix (when (looking-back "foo\\>")
>>>> ;;               (match-string 0)))
>>>> -;;     (candidates (list "foobar" "foobaz" "foobarbaz"))
>>>> -;;     (meta (format "This value is named %s" arg))))
>>>> +;;     (`candidates (list "foobar" "foobaz" "foobarbaz"))
>>>> +;;     (`meta (format "This value is named %s" arg))))
>>> Like the header in company.el says, we still try to support Emacs 22 and
>>> 23. `pcase' was only added in 23.3, I believe.
>> But this is in a comment, so it's not a problem.
> I'm not sure recommending to users to write new backends in a way
> incompatible with older Emacs is good.

It is in my interest to encourage all users to move to a more
recent Emacs.  And even more so to encourage them to move away from `cl'
(which will be a long and arduous path, so every bit helps).

> I guess I'll remove `(init nil)', then.
> By the way, if you think that `init' is useless for all backends, I don't
> believe that's true for `ropemacs' and `clang'. Retrying initialization once
> per buffer is fairly useful.

There are many ways to do initialization in the backend (once per
session, once per buffer, once per blue moon, you name it) without using
`init'.

>> `completion-extra-properties' can have anything you want in it.
>> Same for `completion-metadata'.  Of course, new entries in these
>> alists/plists won't be understood by other users of
>> completion-at-point-functions, but that's not a problem in itself.
> Ah okay, so I guess that section has everything to do with moving some
> company backends to completion-at-point functions.

Very much so, yes.

> This is totally fine for `company-capf', but `company-begin-with' is an old
> function/feature.

But I'm only suggesting to change it in more recent versions of Company,
so it won't affect old users.


        Stefan

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Dmitry Gutov]

On 06.05.2013 5:18, Stefan Monnier wrote:
>>>> Like the header in company.el says, we still try to support Emacs 22 and
>>>> 23. `pcase' was only added in 23.3, I believe.
>>> But this is in a comment, so it's not a problem.
>> I'm not sure recommending to users to write new backends in a way
>> incompatible with older Emacs is good.
>
> It is in my interest to encourage all users to move to a more
> recent Emacs.

I still don't like the dissonance between the "Compatibility" header and 
this recommendation, not far below it. But if you insist, let's keep it 
that way.

There are other reasons to install the latest Emacs, like packages using 
Emacs-24-only features. It doesn't seem to me that Company can really 
benefit from many of those, so far.

> And even more so to encourage them to move away from `cl'
> (which will be a long and arduous path, so every bit helps).

While we're on the subject of `cl', why not use `cl-lib' and `cl-case'? 
I seriously doubt any backend will be able to use the additional 
features provided by `pcase'.

> There are many ways to do initialization in the backend (once per
> session, once per buffer, once per blue moon, you name it) without using
> `init'.

I guess that is true. And if you're just arguing against having a 
similar field in `completion-extra-properties', I'm fine with that.

>> This is totally fine for `company-capf', but `company-begin-with' is an old
>> function/feature.
>
> But I'm only suggesting to change it in more recent versions of Company,
> so it won't affect old users.

That doesn't make sense. Keeping compatibility with Emacs < 24 by 
definition means not breaking it in the latest version.

package.el doesn't have an easy way to install a non-latest version of a 
package, so it will affect any users with old Emacs who are updating or 
reinstalling the package. Provided they sometimes use 
`company-begin-with', of course.

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Stefan Monnier]

>>>>> Like the header in company.el says, we still try to support Emacs 22 and
>>>>> 23. `pcase' was only added in 23.3, I believe.
>>>> But this is in a comment, so it's not a problem.
>>> I'm not sure recommending to users to write new backends in a way
>>> incompatible with older Emacs is good.
>> It is in my interest to encourage all users to move to a more
>> recent Emacs.
> I still don't like the dissonance between the "Compatibility" header and
> this recommendation, not far below it. But if you insist, let's keep it
> that way.

I definitely do not insist on it.  I just explain why I committed the
code this way.  But as maintainer of company, you're free to make
a different decision.

>> And even more so to encourage them to move away from `cl'
>> (which will be a long and arduous path, so every bit helps).
> While we're on the subject of `cl', why not use `cl-lib' and `cl-case'?

No strong reason:
1- cl-lib is even more bleeding-edge.
2- I wrote pcase, so I like pcase.

> I guess that is true.  And if you're just arguing against having a similar
> field in `completion-extra-properties', I'm fine with that.

That's indeed the point.

> package.el doesn't have an easy way to install a non-latest version
> of a package, so it will affect any users with old Emacs who are updating or
> reinstalling the package. Provided they sometimes use `company-begin-with',
> of course.

GNU ELPA is mostly meant for Emacs>=24.  We don't actively try to
prevent users of Emacs<24 from using GNU ELPA, but several of GNU ELPA
packages only work in Emacs>=24.

We can/should add a package-require for (emacs "24.1") or whichever
version is needed, so that package.el won't automatically upgrade to
a package that doesn't work in the currently running Emacs.


        Stefan

Re: /srv/bzr/emacs/elpa r395: * company.el (company-capf): Add support for `sorted' and `post-completion'.

[Dmitry Gutov]

On 06.05.2013 7:22, Stefan Monnier wrote:
>>>>>> Like the header in company.el says, we still try to support Emacs 22 and
>>>>>> 23. `pcase' was only added in 23.3, I believe.
>>>>> But this is in a comment, so it's not a problem.
>>>> I'm not sure recommending to users to write new backends in a way
>>>> incompatible with older Emacs is good.
>>> It is in my interest to encourage all users to move to a more
>>> recent Emacs.
>> I still don't like the dissonance between the "Compatibility" header and
>> this recommendation, not far below it. But if you insist, let's keep it
>> that way.
>
> I definitely do not insist on it.  I just explain why I committed the
> code this way.  But as maintainer of company, you're free to make
> a different decision.

Ok, thanks. I reverted this and the other bit that explicitly required 
lexical binding.
I'll be sure to bring them back when we say goodbye to Emacs 23.

>> package.el doesn't have an easy way to install a non-latest version
>> of a package, so it will affect any users with old Emacs who are updating or
>> reinstalling the package. Provided they sometimes use `company-begin-with',
>> of course.
>
> GNU ELPA is mostly meant for Emacs>=24.  We don't actively try to
> prevent users of Emacs<24 from using GNU ELPA, but several of GNU ELPA
> packages only work in Emacs>=24.

That just means that discontinuing support for Emacs 23 is allowed.

> We can/should add a package-require for (emacs "24.1") or whichever
> version is needed, so that package.el won't automatically upgrade to
> a package that doesn't work in the currently running Emacs.

Sure. js2-mode, for example, already does that.