Subject: Re: bug#70725: 29.3; dired-do-touch completion

From: Thierry Volpiatto <[email-masked]>
Date: Wed, 22 May 2024 04:31:56 +0000

Thierry Volpiatto <[email-masked]> writes:

> Juri Linkov <[email-masked]> writes:
>
>>>>>>>> > However this doesn't explain why dired-do-touch uses a completing-read
>>>>>>>>
>>>>>>>> Indeed, this was an oversight.  Here is the patch
>>>>>>>> that replaces 'completing-read' with 'read-string':
>>>>>>>
>>>>>>> Thierry, is this solution okay with you?
>>>>>>
>>>>>> This fix one issue,
>>>>>
>>>>> Thanks, so I pushed the fix.
>>>>
>>>> Thanks.
>>>>
>>>>>> but default is still wrong IMHO:
>>>>>>
>>>>>> When pressing RET with an empty prompt the value is different than what
>>>>>> is inserted in minibuffer with M-n.  Why do we bother setting the
>>>>>> timesamp at the exact time when pressing RET instead of when pressing
>>>>>> "T", I mean user would consider the timestamp is set once "T" is
>>>>>> pressed, with this the behavior would be consistent with RET and M-n and
>>>>>> the code much simpler.
>>>>>
>>>>> There is no need to make the value used by RET and the value inserted by M-n
>>>>> consistent in 100% of cases.
>>>>
>>>> Sorry but I disagree on this.
>>>
>>> Same question as with previous issue:
>>>
>>> How do I guess (as a third party package maintainer) what DEFAULT is if
>>> you do such things in Emacs?
>>>
>>> We had a similar bug recently where a completing-read was specifying the
>>> default in prompt (with format-prompt) but the DEFAULT arg was not
>>> provided, instead DEFAULT was computed later in the function... How do I
>>> guess what DEFAULT is in such cases? From the prompt? This is not a
>>> valid solution, like this issue prove.
>>
>> The docstring of 'read-string' says:
>>
>>   Fourth arg DEFAULT-VALUE is the default value or the list of default values.
>>    If non-nil, it is used for history commands, and as the value (or the first
>>    element of the list of default values) to return if the user enters the
>>    empty string.
>>
>> So it never returns an empty string.  It always returns the default value
>> that is quite confusing in this case.
>>
>> OTOH, the docstring of 'read-from-minibuffer' says:
>>
>>   Sixth arg DEFAULT-VALUE, if non-nil, should be a string, which is used
>>     as the default to read if READ is non-nil and the user enters
>>     empty input.  But if READ is nil, this function does _not_ return
>>     DEFAULT-VALUE for empty input!  Instead, it returns the empty string.
>>
>> Unlike 'read-string', 'read-from-minibuffer' does not return
>> the default value for empty input.
>>
>> So indeed it would be clearer to use 'read-from-minibuffer'
>> instead of 'read-string' to return an empty string for RET.
>> This is now fixed as well.
>
> In why returning an empty string fix the issue? We are now back at
> initial point, no?

And to add to the complexity, the code below works by chance when using
M-n because it compare two strings with eq.

        (unless (or (string-equal new-attribute "")
        					  ;; Use `eq' instead of `equal'
        					  ;; to detect empty input (bug#12399).
        					  (eq new-attribute default))
        				(if (eq op-symbol 'touch)
        				    (list "-t" new-attribute)
        				  (list new-attribute)))

-- 
Thierry