[Utkarsh Singh] Re: [PATCH] emacs: Add more front ends for address completion

[Utkarsh Singh] Re: [PATCH] emacs: Add more front ends for address completion

[Utkarsh Singh]

-------------------- Start of forwarded message --------------------
From: Utkarsh Singh <utkarsh190601@gmail.com>
To: Tomi Ollila <tomi.ollila@iki.fi>
Subject: Re: [PATCH] emacs: Add more front ends for address completion
Date: Thu, 10 Feb 2022 08:58:49 +0530

Hello Tomi,

On 2022-02-09, 23:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Tue, Feb 08 2022, Utkarsh Singh wrote:
>
>> Hello maintainers,
>>
>> Emacs Lisp Package Archive (ELPA) now includes a package called 'corfu',
>> according to its documentation:
>>
>>         Corfu enhances the default completion in region function with a
>>         completion overlay. The current candidates are shown in a popup
>>         below or above the point.  Corfu is the minimalistic
>>         ~completion-in-region~ counterpart of the
>>         [[https://github.com/minad/vertico][Vertico]] minibuffer UI.
>>
>> Hence, this patch tries to add support for `completion-in-region' in
>> `notmuch-address-expand-name'.  By default, this behaviour is turned off
>> so that existing users can enjoy existing completion techniques.
>
> The current "default" (i.e. w/o any notmuch emacs mua configuration) is to
> use completing-read to do the completion. If "company" is available, then
> company is used by default (w/ all address harvesting and so on...). 
>
> This is "messy" enough ;( (i.e the notmuch-address-selection-function
> is called if company mode is not available or notmuch-address-command
> is a string instead of 'internal or 'as-is (or whatnot, too tired to do
> deep investigation there ;/)
>
> This change, contributes even more "complexity" there. To keep the
> complexity to the same level would be adding more
> notmuch-address-selection-functions and have the defcustom there list
> the options (also probably the name of notmuch-address-selection-function
> would need to be changed to notmuch-fallback-address-selection-function
> ;/)
>

I think, in general, you're right about the complexity and we should try
minimize it. But this patch was originally derived from a bug I was
experiencing with `(global-corfu-mode 1)' in `notmuch-message-mode'.
Here are the steps to reproduce the bug:

1. [Install](https://notmuchmail.org/#index7h2) `notmuch`.
2. In emacs -Q session, evaluate the following

```elisp
(progn
(add-to-list 'load-path "/usr/share/emacs/site-lisp") ; Notmuch Emacs interface
(package-initialize)
(package-install 'corfu)
(package-install 'vertico)

(require 'notmuch)
(require 'corfu)
(require 'vertico)

(vertico-mode 1)
(corfu-global-mode 1)
(notmuch-mua-new-mail))
```

3. Press `TAB` to generate address completion, then press `RET` to
select address of your choice.  After selection, you will notice that
`notmuch-address-expand-name` fails to insert desired address, that is,
you will get the following:

```
To:    
```

Note: Check the trailing spaces.

> All this said, I think this is not simple to solve, as this otherwise fine
> change would indicate :/

No problem, I'm willing to help the maintainers on this matter.

-- 
Utkarsh Singh
https://utkarshsingh.xyz/
-------------------- End of forwarded message --------------------

-- 
Utkarsh Singh
https://utkarshsingh.xyz/

Re: [Utkarsh Singh] Re: [PATCH] emacs: Add more front ends for address completion

[Tomi Ollila]

On Thu, Feb 10 2022, Utkarsh Singh wrote:

> -------------------- Start of forwarded message --------------------
> From: Utkarsh Singh <utkarsh190601@gmail.com>
> To: Tomi Ollila <tomi.ollila@iki.fi>
> Subject: Re: [PATCH] emacs: Add more front ends for address completion
> Date: Thu, 10 Feb 2022 08:58:49 +0530
>
> Hello Tomi,
>
> On 2022-02-09, 23:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
>> On Tue, Feb 08 2022, Utkarsh Singh wrote:
>>
>>> Hello maintainers,
>>>
>>> Emacs Lisp Package Archive (ELPA) now includes a package called 'corfu',
>>> according to its documentation:
>>>
>>>         Corfu enhances the default completion in region function with a
>>>         completion overlay. The current candidates are shown in a popup
>>>         below or above the point.  Corfu is the minimalistic
>>>         ~completion-in-region~ counterpart of the
>>>         [[https://github.com/minad/vertico][Vertico]] minibuffer UI.
>>>
>>> Hence, this patch tries to add support for `completion-in-region' in
>>> `notmuch-address-expand-name'.  By default, this behaviour is turned off
>>> so that existing users can enjoy existing completion techniques.
>>
>> The current "default" (i.e. w/o any notmuch emacs mua configuration) is to
>> use completing-read to do the completion. If "company" is available, then
>> company is used by default (w/ all address harvesting and so on...). 
>>
>> This is "messy" enough ;( (i.e the notmuch-address-selection-function
>> is called if company mode is not available or notmuch-address-command
>> is a string instead of 'internal or 'as-is (or whatnot, too tired to do
>> deep investigation there ;/)
>>
>> This change, contributes even more "complexity" there. To keep the
>> complexity to the same level would be adding more
>> notmuch-address-selection-functions and have the defcustom there list
>> the options (also probably the name of notmuch-address-selection-function
>> would need to be changed to notmuch-fallback-address-selection-function
>> ;/)
>>
>
> I think, in general, you're right about the complexity and we should try
> minimize it. But this patch was originally derived from a bug I was
> experiencing with `(global-corfu-mode 1)' in `notmuch-message-mode'.

Is global-corfu-mode changing how completing-read works ?

> Here are the steps to reproduce the bug:
>
> 1. [Install](https://notmuchmail.org/#index7h2) `notmuch`.
> 2. In emacs -Q session, evaluate the following

When doing in notmuch repo, one can execute ./devel/try-emacs-mua -Q

... which probably uses the same package database (under $HOME) as any
other emacs invocation would do -- what is the way to change the location
of that in order to have chance to have "empty" set of externally installed
packages, and messing with it would not touch the "default" packages... 

(so that I can try the steps below)

Tomi

>
> ```elisp
> (progn
> (add-to-list 'load-path "/usr/share/emacs/site-lisp") ; Notmuch Emacs interface
> (package-initialize)
> (package-install 'corfu)
> (package-install 'vertico)
>
> (require 'notmuch)
> (require 'corfu)
> (require 'vertico)
>
> (vertico-mode 1)
> (corfu-global-mode 1)
> (notmuch-mua-new-mail))
> ```
>
> 3. Press `TAB` to generate address completion, then press `RET` to
> select address of your choice.  After selection, you will notice that
> `notmuch-address-expand-name` fails to insert desired address, that is,
> you will get the following:
>
> ```
> To:    
> ```
>
> Note: Check the trailing spaces.
>
>> All this said, I think this is not simple to solve, as this otherwise fine
>> change would indicate :/
>
> No problem, I'm willing to help the maintainers on this matter.
>
> -- 
> Utkarsh Singh
> https://utkarshsingh.xyz/
> -------------------- End of forwarded message --------------------
>
> -- 
> Utkarsh Singh
> https://utkarshsingh.xyz/
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

Re: [Utkarsh Singh] Re: [PATCH] emacs: Add more front ends for address completion

[Utkarsh Singh]

On 2022-02-10, 19:37 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Thu, Feb 10 2022, Utkarsh Singh wrote:
>
>> -------------------- Start of forwarded message --------------------
>> From: Utkarsh Singh <utkarsh190601@gmail.com>
>> To: Tomi Ollila <tomi.ollila@iki.fi>
>> Subject: Re: [PATCH] emacs: Add more front ends for address completion
>> Date: Thu, 10 Feb 2022 08:58:49 +0530
>>
>> Hello Tomi,
>>
>> On 2022-02-09, 23:59 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>
>>> On Tue, Feb 08 2022, Utkarsh Singh wrote:
>>>
>>>> Hello maintainers,
>>>>
>>>> Emacs Lisp Package Archive (ELPA) now includes a package called 'corfu',
>>>> according to its documentation:
>>>>
>>>>         Corfu enhances the default completion in region function with a
>>>>         completion overlay. The current candidates are shown in a popup
>>>>         below or above the point.  Corfu is the minimalistic
>>>>         ~completion-in-region~ counterpart of the
>>>>         [[https://github.com/minad/vertico][Vertico]] minibuffer UI.
>>>>
>>>> Hence, this patch tries to add support for `completion-in-region' in
>>>> `notmuch-address-expand-name'.  By default, this behaviour is turned off
>>>> so that existing users can enjoy existing completion techniques.
>>>
>>> The current "default" (i.e. w/o any notmuch emacs mua configuration) is to
>>> use completing-read to do the completion. If "company" is available, then
>>> company is used by default (w/ all address harvesting and so on...). 
>>>
>>> This is "messy" enough ;( (i.e the notmuch-address-selection-function
>>> is called if company mode is not available or notmuch-address-command
>>> is a string instead of 'internal or 'as-is (or whatnot, too tired to do
>>> deep investigation there ;/)
>>>
>>> This change, contributes even more "complexity" there. To keep the
>>> complexity to the same level would be adding more
>>> notmuch-address-selection-functions and have the defcustom there list
>>> the options (also probably the name of notmuch-address-selection-function
>>> would need to be changed to notmuch-fallback-address-selection-function
>>> ;/)
>>>
>>
>> I think, in general, you're right about the complexity and we should try
>> minimize it. But this patch was originally derived from a bug I was
>> experiencing with `(global-corfu-mode 1)' in `notmuch-message-mode'.
>
> Is global-corfu-mode changing how completing-read works ?

In theory, no!

>> Here are the steps to reproduce the bug:
>>
>> 1. [Install](https://notmuchmail.org/#index7h2) `notmuch`.
>> 2. In emacs -Q session, evaluate the following
>
> When doing in notmuch repo, one can execute ./devel/try-emacs-mua -Q
>
> ... which probably uses the same package database (under $HOME) as any
> other emacs invocation would do -- what is the way to change the location
> of that in order to have chance to have "empty" set of externally installed
> packages, and messing with it would not touch the "default" packages... 

Okay, we can use `package-user-dir', try the following steps:

1. Press M-w (`kill-ring-save') to kill/cut the following expression:

--8<---------------cut here---------------start------------->8---
(progn
  (setq package-user-dir "/tmp/elpa")
  (package-initialize)
  (package-install 'corfu)
  (package-install 'vertico)

  (require 'notmuch)
  (require 'corfu)
  (require 'vertico)

  (vertico-mode 1)
  (corfu-global-mode 1)
  (notmuch-mua-new-mail))
--8<---------------cut here---------------end--------------->8---

2. Now in ./devel/try-emacs-mua -Q, press M-: (`eval-expression'), C-y
(`yank') and finally RET.

3. Press TAB to generate address completion, then press RET to select a
address, for eg. "Notmuch mailing list <notmuch@notmuchmail.org>".
After selection, you will notice that `notmuch-address-expand-name'
insert desired address with trailing whitespaces.

--8<---------------cut here---------------start------------->8---
To: Notmuch mailing list <notmuch@notmuchmail.org>      
--8<---------------cut here---------------end--------------->8---

I apologize as results are not as reproducible as I thought, but you can
still notice that `notmuch-address-expand-name' is not producing the
desired result.

-- 
Utkarsh Singh
https://utkarshsingh.xyz/

Re: [Utkarsh Singh] Re: [PATCH] emacs: Add more front ends for address completion

[Tomi Ollila]

On Fri, Feb 11 2022, Utkarsh Singh wrote:

> On 2022-02-10, 19:37 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
>>
>> When doing in notmuch repo, one can execute ./devel/try-emacs-mua -Q
>>
>> ... which probably uses the same package database (under $HOME) as any
>> other emacs invocation would do -- what is the way to change the location
>> of that in order to have chance to have "empty" set of externally installed
>> packages, and messing with it would not touch the "default" packages... 
>
> Okay, we can use `package-user-dir', try the following steps:
>
> 1. Press M-w (`kill-ring-save') to kill/cut the following expression:
>
> --8<---------------cut here---------------start------------->8---
> (progn
>   (setq package-user-dir "/tmp/elpa")
>   (package-initialize)
>   (package-install 'corfu)
>   (package-install 'vertico)
>
>   (require 'notmuch)
>   (require 'corfu)
>   (require 'vertico)
>
>   (vertico-mode 1)
>   (corfu-global-mode 1)
>   (notmuch-mua-new-mail))
> --8<---------------cut here---------------end--------------->8---
>
> 2. Now in ./devel/try-emacs-mua -Q, press M-: (`eval-expression'), C-y
> (`yank') and finally RET.
>
> 3. Press TAB to generate address completion, then press RET to select a
> address, for eg. "Notmuch mailing list <notmuch@notmuchmail.org>".
> After selection, you will notice that `notmuch-address-expand-name'
> insert desired address with trailing whitespaces.

Yes, I see trailing whitespace.
Also, completion matches only from beginning of the address string.
Also, after one completion TAB no longer completes...

Tomi


>
> --8<---------------cut here---------------start------------->8---
> To: Notmuch mailing list <notmuch@notmuchmail.org>      
> --8<---------------cut here---------------end--------------->8---
>
> I apologize as results are not as reproducible as I thought, but you can
> still notice that `notmuch-address-expand-name' is not producing the
> desired result.
>
> -- 
> Utkarsh Singh
> https://utkarshsingh.xyz/