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

From: Thierry Volpiatto <[email-masked]>
Date: Thu, 23 May 2024 15:42:35 +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?
>>
>> No, the accidental completing-read that caused you trouble
>> was removed completely, so your problem is solved now.
>
> Yes it is solved, thank you, but I was speaking of the M-n behavior vs
> RET.  It can be easily fixed with a default set to ("now"
> current_file_timestamp) where "now" is the string "now", but I will now
> stop posting about this.

Here a possible patch in case you are interested to fix this (fully not
tested as I am on 29.3 actually):

diff --git a/lisp/dired-aux.el b/lisp/dired-aux.el
index 22c6881ae35..954cfdd27cb 100644
--- a/lisp/dired-aux.el
+++ b/lisp/dired-aux.el
@@ -434,10 +434,11 @@ List has a form of (file-name full-file-name (attribute-list))."
 	 (default-file (dired-get-filename t t))
 	 (default (when default-file
 		    (cond ((eq op-symbol 'touch)
-			   (format-time-string
-			    "%Y%m%d%H%M.%S"
-			    (file-attribute-modification-time
-			     (file-attributes default-file))))
+			   (list "now"
+                                 (format-time-string
+			          "%Y%m%d%H%M.%S"
+			          (file-attribute-modification-time
+			           (file-attributes default-file)))))
 			  ((eq op-symbol 'chown)
 			   (file-attribute-user-id
 			    (file-attributes default-file 'string)))
@@ -473,8 +474,9 @@ List has a form of (file-name full-file-name (attribute-list))."
 					  ;; Use `eq' instead of `equal'
 					  ;; to detect empty input (bug#12399).
 					  (eq new-attribute default))
-				(if (eq op-symbol 'touch)
-				    (list "-t" new-attribute)
+				(if (and (eq op-symbol 'touch)
+				         (not (string= new-attribute "now")))
+                                    (list "-t" new-attribute)
 				  (list new-attribute)))
 			      (if (string-match-p "gnu" system-configuration)
 				  '("--") nil))

-- 
Thierry