Re: master 9904376c797: Support calling 'project-current' with custom prompt

Re: master 9904376c797: Support calling 'project-current' with custom prompt

[Eshel Yaron]

Hi Dmitry,

Dmitry Gutov <dgutov@yandex.ru> writes:

> branch: master
> commit 9904376c797665de47ff760bcf8c2fe33d7ae625
> Author: Dmitry Gutov <dmitry@gutov.dev>
> Commit: Dmitry Gutov <dmitry@gutov.dev>
>
>     Support calling 'project-current' with custom prompt
>     

[...]

> -      (setq directory (funcall project-prompter)
> +      (setq directory (if (stringp maybe-prompt)
> +                          (funcall project-prompter maybe-prompt)
> +                        (funcall project-prompter))

JFYI, I'm using a custom project-prompter, which was not prepared to be
called with an argument, so I got some unexpected errors until I
realized what changed.  A quick lookup online shows that although this
option is quite new, a few others already set it to a custom function,
so it might be worth catching the "Wrong number of arguments" error.

Also, I'd like to update the docstring of project-prompter along the
following lines, if it looks right to you:

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 3cdaa7c2a76..6383bdc95d2 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -199,7 +199,9 @@ project-current-directory-override
 
 (defcustom project-prompter #'project-prompt-project-dir
   "Function to call to prompt for a project.
-Called with no arguments and should return a project root dir."
+The function is either called with no arguments or with one argument,
+which is the prompt string to use when prompting.  It should return a
+project root directory."
   :type '(choice (const :tag "Prompt for a project directory"
                         project-prompt-project-dir)
                  (const :tag "Prompt for a project name"


Cheers,

Eshel

Re: master 9904376c797: Support calling 'project-current' with custom prompt

[Dmitry Gutov]

Hi Eshel,

On 23/10/2024 09:54, Eshel Yaron wrote:
> Hi Dmitry,
> 
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> branch: master
>> commit 9904376c797665de47ff760bcf8c2fe33d7ae625
>> Author: Dmitry Gutov <dmitry@gutov.dev>
>> Commit: Dmitry Gutov <dmitry@gutov.dev>
>>
>>      Support calling 'project-current' with custom prompt
>>      
> 
> [...]
> 
>> -      (setq directory (funcall project-prompter)
>> +      (setq directory (if (stringp maybe-prompt)
>> +                          (funcall project-prompter maybe-prompt)
>> +                        (funcall project-prompter))
> 
> JFYI, I'm using a custom project-prompter, which was not prepared to be
> called with an argument, so I got some unexpected errors until I
> realized what changed.  A quick lookup online shows that although this
> option is quite new, a few others already set it to a custom function,
> so it might be worth catching the "Wrong number of arguments" error.

It's good of you to bring this up, but under what conditions do you 
encounter the breakage? Are there 3rd-party callers of 'project-current' 
already as well, that pass a string as MAYBE-PROMPT? If so, that's a 
little quicker than I expected. :)

Catching an error might be a fine approach, but then a lot of people 
would miss out on the reason why the provided prompt is not used.

> Also, I'd like to update the docstring of project-prompter along the
> following lines, if it looks right to you:
> 
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 3cdaa7c2a76..6383bdc95d2 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -199,7 +199,9 @@ project-current-directory-override
>   
>   (defcustom project-prompter #'project-prompt-project-dir
>     "Function to call to prompt for a project.
> -Called with no arguments and should return a project root dir."
> +The function is either called with no arguments or with one argument,
> +which is the prompt string to use when prompting.  It should return a
> +project root directory."
>     :type '(choice (const :tag "Prompt for a project directory"
>                           project-prompt-project-dir)
>                    (const :tag "Prompt for a project name"

Thanks, I've pushed a slight edit of that (sorry it went out without 
attribution). Also updated the corresponding NEWS entry.

Re: master 9904376c797: Support calling 'project-current' with custom prompt

[Eshel Yaron]

Dmitry Gutov <dmitry@gutov.dev> writes:

>> JFYI, I'm using a custom project-prompter, which was not prepared to
>> be
>> called with an argument, so I got some unexpected errors until I
>> realized what changed.  A quick lookup online shows that although this
>> option is quite new, a few others already set it to a custom function,
>> so it might be worth catching the "Wrong number of arguments" error.
>
> It's good of you to bring this up, but under what conditions do you
> encounter the breakage? Are there 3rd-party callers of
> 'project-current' already as well, that pass a string as MAYBE-PROMPT?
> If so, that's a little quicker than I expected. :)

Ah, no, you're right, I got the error through custom code of mine.
I adapted some code to call project-current with a string, without
adapting my project-prompter function accordingly.

> Catching an error might be a fine approach, but then a lot of people
> would miss out on the reason why the provided prompt is not used.

Yeah, that's a reasonable consideration too.

>> Also, I'd like to update the docstring of project-prompter along the
>> following lines, if it looks right to you:
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index 3cdaa7c2a76..6383bdc95d2 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -199,7 +199,9 @@ project-current-directory-override
>>     (defcustom project-prompter #'project-prompt-project-dir
>>     "Function to call to prompt for a project.
>> -Called with no arguments and should return a project root dir."
>> +The function is either called with no arguments or with one argument,
>> +which is the prompt string to use when prompting.  It should return a
>> +project root directory."
>>     :type '(choice (const :tag "Prompt for a project directory"
>>                           project-prompt-project-dir)
>>                    (const :tag "Prompt for a project name"
>
> Thanks, I've pushed a slight edit of that (sorry it went out without
> attribution). Also updated the corresponding NEWS entry.

Great, thank you.


Eshel