bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Óscar Fuentes]

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]


2014-11-13 Oscar Fuentes <ofv@wanadoo.es>

	When showing a log with git, continue beyond renames.
	* lisp/vc/vc-git.el (vc-git-print-log): Use --follow.



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-git-log-follow-renames.patch --]
[-- Type: text/x-diff, Size: 455 bytes --]

diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index afcfd66..19c31e1 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -839,6 +839,7 @@ If LIMIT is non-nil, show no more than this many entries."
                     ,(format "--pretty=tformat:%s"
 			     (car vc-git-root-log-format))
 		    "--abbrev-commit"))
+		'("--follow")
 		(when limit (list "-n" (format "%s" limit)))
 		(when start-revision (list start-revision))
 		'("--")))))))

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

ofv@wanadoo.es (Óscar Fuentes) writes:

> 2014-11-13 Oscar Fuentes <ofv@wanadoo.es>
>
> 	When showing a log with git, continue beyond renames.
> 	* lisp/vc/vc-git.el (vc-git-print-log): Use --follow.

You should check out the discussion at http://debbugs.gnu.org/8756.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Óscar Fuentes]

Dmitry Gutov <dgutov@yandex.ru> writes:

> You should check out the discussion at http://debbugs.gnu.org/8756.

Ok, so it is not as simple as it seems.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 309 bytes --]

Too bad that a new useful customizable variable
vc-git-print-log-follow is not mentioned in NEWS.26.

It works quite well, but only with the following fix
that puts more restrictions - it checks not only
for a directory, but also if it is a single file.

The whole patch is intended for the emacs-26 branch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-git-print-log-follow.patch --]
[-- Type: text/x-diff, Size: 1491 bytes --]

diff --git a/etc/NEWS b/etc/NEWS
index 5b3e445..6aa4b2d 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1169,6 +1169,9 @@ branch-related commands on a keymap bound to 'B'.
 *** 'vc-region-history' is now bound to 'C-x v h', replacing the older
 'vc-insert-headers' binding.
 
+*** New user option 'vc-git-print-log-follow' follow renames in Git logs
+for a single file.
+
 ** CC mode
 
 ---
diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
index 3bf837c..9b54b88 100644
--- a/lisp/vc/vc-git.el
+++ b/lisp/vc/vc-git.el
@@ -994,7 +994,7 @@ This prompts for a branch to merge from."
 (autoload 'vc-setup-buffer "vc-dispatcher")
 
 (defcustom vc-git-print-log-follow nil
-  "If true, follow renames in Git logs for files."
+  "If true, follow renames in Git logs for a single file."
   :type 'boolean
   :version "26.1")
 
@@ -1019,8 +1019,9 @@ If LIMIT is non-nil, show no more than this many entries."
 	       (append
 		'("log" "--no-color")
                 (when (and vc-git-print-log-follow
-                           (not (cl-some #'file-directory-p files)))
-                  ;; "--follow" on directories is broken
+                           (null (cdr files))
+                           (not (file-directory-p (car files))))
+                  ;; "--follow" on directories or multiple files is broken
                   ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756
                   ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=16422
                   (list "--follow"))

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

> Too bad that a new useful customizable variable
> vc-git-print-log-follow is not mentioned in NEWS.26.
>
> It works quite well, but only with the following fix
> that puts more restrictions - it checks not only
> for a directory, but also if it is a single file.
>
> The whole patch is intended for the emacs-26 branch:

Pushed to the emacs-26 branch.

Here is a related problem.  The executed git command is not shown
completely in the echo area, this is ok since there is not much space.

But the git command is truncated in the *Messages* buffer, this is bad
because sometimes there is a need to inspect the command run by git with
its command line switches in its entirety, especially in case of errors.

The command string is truncated in ‘vc-do-command’ and let-bound to
‘full-command’, so we need to add an untruncated command string
to the *Messages* buffer.

Here is an example of what will be in *Messages* after this patch:

  Running git --no-pager log -p -L1001,1039:vc-git.el
  Running git --no-pager log -p -L... . in background...
  Running git --no-pager log -p -L... . in background... done

This patch is for the master branch:

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index b0d2221..efdd29e 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -313,6 +313,8 @@ vc-do-command
 	    (status 0))
 	(when files
 	  (setq squeezed (nconc squeezed files)))
+        (let ((inhibit-message t))
+          (message "Running %s %s" command (vc-delistify squeezed)))
 	(let (;; Since some functions need to parse the output
 	      ;; from external commands, set LC_MESSAGES to C.
 	      (process-environment (cons "LC_MESSAGES=C" process-environment))

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/5/18 00:58, Juri Linkov wrote:

> The command string is truncated in ‘vc-do-command’ and let-bound to
> ‘full-command’, so we need to add an untruncated command string
> to the *Messages* buffer.

Looks okay to me. But here's an alternative to consider:

Could we use message-truncate-lines and just one message call instead?

The 20 characters limit seems arbitrary, we just need to avoid resizing 
the echo area. And if the "in background... done" part being truncated 
is a worry, we can rephrase them to go at the beginning of the sentence.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 04 Feb 2018 23:58:53 +0200
> 
> > Too bad that a new useful customizable variable
> > vc-git-print-log-follow is not mentioned in NEWS.26.
> >
> > It works quite well, but only with the following fix
> > that puts more restrictions - it checks not only
> > for a directory, but also if it is a single file.
> >
> > The whole patch is intended for the emacs-26 branch:
> 
> Pushed to the emacs-26 branch.

Please don't push anything to the release branch without John's or
mine explicit approval.  As things are, I'm very busy these days, and
didn't yet have time to review the patch thoroughly, so I'm not sure
I'm happy with its being pushed.

> diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
> index b0d2221..efdd29e 100644
> --- a/lisp/vc/vc-dispatcher.el
> +++ b/lisp/vc/vc-dispatcher.el
> @@ -313,6 +313,8 @@ vc-do-command
>  	    (status 0))
>  	(when files
>  	  (setq squeezed (nconc squeezed files)))
> +        (let ((inhibit-message t))
> +          (message "Running %s %s" command (vc-delistify squeezed)))
>  	(let (;; Since some functions need to parse the output
>  	      ;; from external commands, set LC_MESSAGES to C.
>  	      (process-environment (cons "LC_MESSAGES=C" process-environment))

I don't understand why would we want to hard-code the fact that these
commands are not shown in the echo area.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 5 Feb 2018 02:31:14 +0300
> 
> The 20 characters limit seems arbitrary, we just need to avoid resizing 
> the echo area.

There are means to prevent resizing the mini-window without truncating
anything in *Messages*.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/5/18 06:39, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Mon, 5 Feb 2018 02:31:14 +0300
>>
>> The 20 characters limit seems arbitrary, we just need to avoid resizing
>> the echo area.
> 
> There are means to prevent resizing the mini-window without truncating
> anything in *Messages*.

message-truncate-lines, right? Which I mentioned.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> Cc: 19045@debbugs.gnu.org, juri@linkov.net
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Mon, 5 Feb 2018 11:25:39 +0300
> 
> > There are means to prevent resizing the mini-window without truncating
> > anything in *Messages*.
> 
> message-truncate-lines, right? Which I mentioned.

No, I meant resize-mini-windows.  The advantage of binding that around
the offending code is that it doesn't affect what is being logged in
*Messages*.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/5/18 19:18, Eli Zaretskii wrote:

>>> There are means to prevent resizing the mini-window without truncating
>>> anything in *Messages*.
>>
>> message-truncate-lines, right? Which I mentioned.
> 
> No, I meant resize-mini-windows.  The advantage of binding that around
> the offending code is that it doesn't affect what is being logged in
> *Messages*.

Oh, I see. I read the former's docstring wrong.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>> > Too bad that a new useful customizable variable
>> > vc-git-print-log-follow is not mentioned in NEWS.26.
>> >
>> > It works quite well, but only with the following fix
>> > that puts more restrictions - it checks not only
>> > for a directory, but also if it is a single file.
>> >
>> > The whole patch is intended for the emacs-26 branch:
>> 
>> Pushed to the emacs-26 branch.
>
> Please don't push anything to the release branch without John's or
> mine explicit approval.  As things are, I'm very busy these days, and
> didn't yet have time to review the patch thoroughly, so I'm not sure
> I'm happy with its being pushed.

Sorry, will wait more time the next time.

>> diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
>> index b0d2221..efdd29e 100644
>> --- a/lisp/vc/vc-dispatcher.el
>> +++ b/lisp/vc/vc-dispatcher.el
>> @@ -313,6 +313,8 @@ vc-do-command
>>  	    (status 0))
>>  	(when files
>>  	  (setq squeezed (nconc squeezed files)))
>> +        (let ((inhibit-message t))
>> +          (message "Running %s %s" command (vc-delistify squeezed)))
>>  	(let (;; Since some functions need to parse the output
>>  	      ;; from external commands, set LC_MESSAGES to C.
>>  	      (process-environment (cons "LC_MESSAGES=C" process-environment))
>
> I don't understand why would we want to hard-code the fact that these
> commands are not shown in the echo area.

I don't want to cause more flicker in the echo area when the full command
gets overwritten immediately by the truncated command in the echo area. 

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

> Could we use message-truncate-lines and just one message call instead?

Maybe.  But currently what is truncated is not the whole command line,
but each of the command switches.  I don't know how important this is.

> The 20 characters limit seems arbitrary, we just need to avoid resizing the
> echo area. And if the "in background... done" part being truncated is
> a worry, we can rephrase them to go at the beginning of the sentence.

Indeed, truncating "in background... done" is not nice, and I have
no idea how to rephrase it.

Anyway, this is the current patch to try:

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index b0d2221..21f2b98 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -290,16 +290,12 @@ vc-do-command
   (let* ((files
 	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
 		  (if (listp file-or-list) file-or-list (list file-or-list))))
+	 (message-truncate-lines t)
 	 (full-command
-	  ;; What we're doing here is preparing a version of the command
-	  ;; for display in a debug-progress message.  If it's fewer than
-	  ;; 20 characters display the entire command (without trailing
-	  ;; newline).  Otherwise display the first 20 followed by an ellipsis.
 	  (concat (if (string= (substring command -1) "\n")
 		      (substring command 0 -1)
 		    command)
-		  " "
-		  (vc-delistify (mapcar (lambda (s) (if (> (length s) 20) (concat (substring s 0 2) "...")  s)) flags))
+		  " " (vc-delistify flags)
 		  " " (vc-delistify files))))
     (save-current-buffer
       (unless (or (eq buffer t)
@@ -332,7 +328,8 @@ vc-do-command
 		(setq status proc)
 		(when vc-command-messages
 		  (vc-run-delayed
-		   (message "Running %s in background... done" full-command))))
+		    (let ((message-truncate-lines t))
+		      (message "Running %s in background... done" full-command)))))
 	    ;; Run synchronously
 	    (when vc-command-messages
 	      (message "Running %s in foreground..." full-command))

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> > Please don't push anything to the release branch without John's or
> > mine explicit approval.  As things are, I'm very busy these days, and
> > didn't yet have time to review the patch thoroughly, so I'm not sure
> > I'm happy with its being pushed.
> 
> Sorry, will wait more time the next time.

Thanks in advance.

> >> +        (let ((inhibit-message t))
> >> +          (message "Running %s %s" command (vc-delistify squeezed)))
> >>  	(let (;; Since some functions need to parse the output
> >>  	      ;; from external commands, set LC_MESSAGES to C.
> >>  	      (process-environment (cons "LC_MESSAGES=C" process-environment))
> >
> > I don't understand why would we want to hard-code the fact that these
> > commands are not shown in the echo area.
> 
> I don't want to cause more flicker in the echo area when the full command
> gets overwritten immediately by the truncated command in the echo area. 

Would binding resize-mini-windows to nil instead fix that?  If it
would, then I think it's a better way, because it doesn't affect what
is being recorded in *Messages*.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/6/18 00:31, Juri Linkov wrote:
>> Could we use message-truncate-lines and just one message call instead?
> 
> Maybe.  But currently what is truncated is not the whole command line,
> but each of the command switches.  I don't know how important this is.

Apparently the idea is that "in background... done" is shown in the echo 
area regardless of the command string length.

>> The 20 characters limit seems arbitrary, we just need to avoid resizing the
>> echo area. And if the "in background... done" part being truncated is
>> a worry, we can rephrase them to go at the beginning of the sentence.
> 
> Indeed, truncating "in background... done" is not nice, and I have
> no idea how to rephrase it.

How about:

Running in background: %s

and

Finished in background: %s

> Anyway, this is the current patch to try:

Does it work like you wanted? According to Eli, we should replace 
message-truncate-lines here with resize-mini-windows.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

> Would binding resize-mini-windows to nil instead fix that?  If it
> would, then I think it's a better way, because it doesn't affect what
> is being recorded in *Messages*.

I created a patch below that let-binds resize-mini-windows to nil,
but it has no effect and always resizes the echo area.
Should I create a separate bug report for this?

Also there is still a problem of hiding the trailing part with
“in background... done” when it is not displayed at the end of
the message, and I have no idea how to reformulate this phrase.

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index b0d2221..a3d8e3d 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -290,16 +290,12 @@ vc-do-command
   (let* ((files
 	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
 		  (if (listp file-or-list) file-or-list (list file-or-list))))
+	 (resize-mini-windows nil)
 	 (full-command
-	  ;; What we're doing here is preparing a version of the command
-	  ;; for display in a debug-progress message.  If it's fewer than
-	  ;; 20 characters display the entire command (without trailing
-	  ;; newline).  Otherwise display the first 20 followed by an ellipsis.
 	  (concat (if (string= (substring command -1) "\n")
 		      (substring command 0 -1)
 		    command)
-		  " "
-		  (vc-delistify (mapcar (lambda (s) (if (> (length s) 20) (concat (substring s 0 2) "...")  s)) flags))
+		  " " (vc-delistify flags)
 		  " " (vc-delistify files))))
     (save-current-buffer
       (unless (or (eq buffer t)
@@ -332,7 +328,8 @@ vc-do-command
 		(setq status proc)
 		(when vc-command-messages
 		  (vc-run-delayed
-		   (message "Running %s in background... done" full-command))))
+		    (let ((resize-mini-windows nil))
+		      (message "Running %s in background... done" full-command)))))
 	    ;; Run synchronously
 	    (when vc-command-messages
 	      (message "Running %s in foreground..." full-command))

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>>> The 20 characters limit seems arbitrary, we just need to avoid resizing the
>>> echo area. And if the "in background... done" part being truncated is
>>> a worry, we can rephrase them to go at the beginning of the sentence.
>>
>> Indeed, truncating "in background... done" is not nice, and I have
>> no idea how to rephrase it.
>
> How about:
>
> Running in background: %s
>
> and
>
> Finished in background: %s

Not bad, better would be to try to find words with the same length,
so the whole command won't jump 1 char to the right when finished.

>> Anyway, this is the current patch to try:
>
> Does it work like you wanted? According to Eli, we should replace
> message-truncate-lines here with resize-mini-windows.

message-truncate-lines works fine whereas resize-mini-windows doesn't.
Maybe the fact that it runs processes has its effect on resizing.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 19045@debbugs.gnu.org
> Date: Tue, 06 Feb 2018 23:04:28 +0200
> 
> I created a patch below that let-binds resize-mini-windows to nil,
> but it has no effect and always resizes the echo area.
> Should I create a separate bug report for this?

No, it's not a bug.  Sorry, I forgot how this feature works: this
variable cannot be let-bound.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>> I created a patch below that let-binds resize-mini-windows to nil,
>> but it has no effect and always resizes the echo area.
>> Should I create a separate bug report for this?
>
> No, it's not a bug.  Sorry, I forgot how this feature works: this
> variable cannot be let-bound.

Then is it ok to install the patch that uses message-truncate-lines?
(to master, of course)

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/7/18 00:51, Juri Linkov wrote:

>> How about:
>>
>> Running in background: %s
>>
>> and
>>
>> Finished in background: %s
> 
> Not bad, better would be to try to find words with the same length,
> so the whole command won't jump 1 char to the right when finished.

If we fail to find better wording, maybe add an extra space between 
"Running" and "in".

> message-truncate-lines works fine whereas resize-mini-windows doesn't.
> Maybe the fact that it runs processes has its effect on resizing.

If it works (and doesn't affect the contents of *Messages*, which was 
the goal), great. That's all we need.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 19045@debbugs.gnu.org
> Date: Thu, 08 Feb 2018 23:09:04 +0200
> 
> >> I created a patch below that let-binds resize-mini-windows to nil,
> >> but it has no effect and always resizes the echo area.
> >> Should I create a separate bug report for this?
> >
> > No, it's not a bug.  Sorry, I forgot how this feature works: this
> > variable cannot be let-bound.
> 
> Then is it ok to install the patch that uses message-truncate-lines?
> (to master, of course)

Yes, thanks.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 03 Feb 2018 23:59:58 +0200
> 
>  (defcustom vc-git-print-log-follow nil
> -  "If true, follow renames in Git logs for files."
> +  "If true, follow renames in Git logs for a single file."
>    :type 'boolean
>    :version "26.1")
>  
> @@ -1019,8 +1019,9 @@ If LIMIT is non-nil, show no more than this many entries."
>  	       (append
>  		'("log" "--no-color")
>                  (when (and vc-git-print-log-follow
> -                           (not (cl-some #'file-directory-p files)))
> -                  ;; "--follow" on directories is broken
> +                           (null (cdr files))
> +                           (not (file-directory-p (car files))))
> +                  ;; "--follow" on directories or multiple files is broken
>                    ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756
>                    ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=16422
>                    (list "--follow"))

What does this mean "is broken" in this context?  Is this a Git
bug/misfeature, or is that vc-git.el problem?  I cannot figure out the
answer by reading the cited bug reports.

And the doc string doesn't sound detailed enough to me, as it doesn't
tell enough about the commands that are affected.

Since this is now on the release branch, we should fix these issues
ASAP.

Thanks.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>> @@ -1019,8 +1019,9 @@ If LIMIT is non-nil, show no more than this many entries."
>>  	       (append
>>  		'("log" "--no-color")
>>                  (when (and vc-git-print-log-follow
>> -                           (not (cl-some #'file-directory-p files)))
>> -                  ;; "--follow" on directories is broken
>> +                           (null (cdr files))
>> +                           (not (file-directory-p (car files))))
>> +                  ;; "--follow" on directories or multiple files is broken
>>                    ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=8756
>>                    ;; https://debbugs.gnu.org/cgi/bugreport.cgi?bug=16422
>>                    (list "--follow"))
>
> What does this mean "is broken" in this context?  Is this a Git
> bug/misfeature, or is that vc-git.el problem?  I cannot figure out the
> answer by reading the cited bug reports.
>
> And the doc string doesn't sound detailed enough to me, as it doesn't
> tell enough about the commands that are affected.

When the argument ‘files’ contains more than one file, then git fails
with the same error as when ‘files’ contains a directory (the case
that was previously handled by ‘cl-some #'file-directory-p’ above).

The detailed recipe for the previous version of vc-git.el is here:

0. emacs -Q
1. C-x d directory_under_Git_control
2. use customization or just (setq vc-git-print-log-follow t)
3. in Dired mark only one file
4. C-x v l   - works ok showing the log
5. mark a second file, so there are 2 marked files in Dired
6. C-x v l   - git fails with the following error message:

  “usage: git logs can only follow renames on one pathname at a time”

in git version 1.9.1

So the fix was to check whether there is only one file given as an argument,
and it is not a directory.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>>> How about:
>>>
>>> Running in background: %s
>>>
>>> and
>>>
>>> Finished in background: %s
>>
>> Not bad, better would be to try to find words with the same length,
>> so the whole command won't jump 1 char to the right when finished.
>
> If we fail to find better wording, maybe add an extra space between
> "Running" and "in".

Yes, an extra space space will help.

In addition to these background messages, we also have to find
better wording for foreground messages as well:

"Running %s in foreground..."
"Running %s...OK = %d"
"Running %s...FAILED (%s)"

where additional argument is status.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: 19045@debbugs.gnu.org
> Date: Sat, 10 Feb 2018 23:08:32 +0200
> 
> 0. emacs -Q
> 1. C-x d directory_under_Git_control
> 2. use customization or just (setq vc-git-print-log-follow t)
> 3. in Dired mark only one file
> 4. C-x v l   - works ok showing the log
> 5. mark a second file, so there are 2 marked files in Dired
> 6. C-x v l   - git fails with the following error message:
> 
>   “usage: git logs can only follow renames on one pathname at a time”
> 
> in git version 1.9.1

Thanks for the example, this works fine here with Git 2.10.0.

> So the fix was to check whether there is only one file given as an argument,
> and it is not a directory.

So I think we should disable multiple files only on old versions of
Git that don't support this.  Do we know which version started
supporting this?  And what about directories?

Thanks.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Andreas Schwab]

On Feb 11 2018, Eli Zaretskii <eliz@gnu.org> wrote:

> So I think we should disable multiple files only on old versions of
> Git that don't support this.  Do we know which version started
> supporting this?  And what about directories?

git log --follow still requires exactly one pathspec (but it can match
multiple files).  A directory is a pathspec that matches all files in
that directory (recursively).

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: Juri Linkov <juri@linkov.net>,  19045@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 18:03:00 +0100
> 
> On Feb 11 2018, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > So I think we should disable multiple files only on old versions of
> > Git that don't support this.  Do we know which version started
> > supporting this?  And what about directories?
> 
> git log --follow still requires exactly one pathspec (but it can match
> multiple files).  A directory is a pathspec that matches all files in
> that directory (recursively).

Thanks.  I see that marking several files indeed fails, but marking a
single directory does work.  Did that use to fail in Git 1.9?

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Andreas Schwab]

On Feb 11 2018, Eli Zaretskii <eliz@gnu.org> wrote:

> Thanks.  I see that marking several files indeed fails, but marking a
> single directory does work.  Did that use to fail in Git 1.9?

I'm not aware of any change in the handling of --follow in connection
with pathspecs.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Andreas Schwab <schwab@linux-m68k.org>
> Cc: juri@linkov.net,  19045@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 19:22:49 +0100
> 
> On Feb 11 2018, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> > Thanks.  I see that marking several files indeed fails, but marking a
> > single directory does work.  Did that use to fail in Git 1.9?
> 
> I'm not aware of any change in the handling of --follow in connection
> with pathspecs.

OK, thanks.

Juri, can you please re-check with directories?  They seem to work
fine here, so disabling them in this case sounds unnecessary.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

> Juri, can you please re-check with directories?  They seem to work
> fine here, so disabling them in this case sounds unnecessary.

Please clarify where directories work fine: in git command line
or in Emacs using vc-git?

In the command line of git version 1.9.1 running git log on two files
(command line copied from *Messages* when vc-command-messages is t)

  git --no-pager log --no-color --follow -n 2000 -- vc-dispatcher.el vc-git.el

fails with the error:

  usage: git logs can only follow renames on one pathname at a time

whereas running git on one directory ‘vc’

  git --no-pager log --no-color --follow -n 2000 -- vc

doesn't fail.  Although I can't confirm the correctness of the output log:
it seems the option ‘--follow’ has no effect on directories.

Now there is a different story for vc-git.el.  There are 4 versions of it:

1. the initial version without the option added in ‘vc-git-print-log’
2. the old version that always used "--follow"
3. the previous version that added "--follow" only
   when the arg ‘files’ contains no directories
4. the current version that additionally checks the length of ‘files’
   and adds "--follow" only for a single file in ‘files’.

Which version did you check?

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/11/18 12:14 AM, Juri Linkov wrote:

> "Running %s in foreground..."

Same as with "background".

> "Running %s...OK = %d"

Success (status 0): ...

> "Running %s...FAILED (%s)"

Failed  (status 3): ...

Something like that. If the exit code is longer than 1 digit (pretty 
rare), maybe remove the extra space after "Failed".

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Andreas Schwab <schwab@linux-m68k.org>,  19045@debbugs.gnu.org
> Date: Sun, 11 Feb 2018 23:22:59 +0200
> 
> > Juri, can you please re-check with directories?  They seem to work
> > fine here, so disabling them in this case sounds unnecessary.
> 
> Please clarify where directories work fine: in git command line
> or in Emacs using vc-git?

The latter.

> 
> In the command line of git version 1.9.1 running git log on two files
> (command line copied from *Messages* when vc-command-messages is t)
> 
>   git --no-pager log --no-color --follow -n 2000 -- vc-dispatcher.el vc-git.el
> 
> fails with the error:
> 
>   usage: git logs can only follow renames on one pathname at a time
> 
> whereas running git on one directory ‘vc’
> 
>   git --no-pager log --no-color --follow -n 2000 -- vc
> 
> doesn't fail.  Although I can't confirm the correctness of the output log:
> it seems the option ‘--follow’ has no effect on directories.

Even if it's true, it means --follow doesn't do any harm when invoked
on directories, and we shouldn't disallow that.

> Now there is a different story for vc-git.el.  There are 4 versions of it:
> 
> 1. the initial version without the option added in ‘vc-git-print-log’
> 2. the old version that always used "--follow"
> 3. the previous version that added "--follow" only
>    when the arg ‘files’ contains no directories
> 4. the current version that additionally checks the length of ‘files’
>    and adds "--follow" only for a single file in ‘files’.
> 
> Which version did you check?

I tried #2.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>> In the command line of git version 1.9.1 running git log on two files
>> (command line copied from *Messages* when vc-command-messages is t)
>> 
>>   git --no-pager log --no-color --follow -n 2000 -- vc-dispatcher.el vc-git.el
>> 
>> fails with the error:
>> 
>>   usage: git logs can only follow renames on one pathname at a time
>> 
>> whereas running git on one directory ‘vc’
>> 
>>   git --no-pager log --no-color --follow -n 2000 -- vc
>> 
>> doesn't fail.  Although I can't confirm the correctness of the output log:
>> it seems the option ‘--follow’ has no effect on directories.
>
> Even if it's true, it means --follow doesn't do any harm when invoked
> on directories, and we shouldn't disallow that.

I don't know the reason why disallowing directories was implemented
in the first place.  Maybe ‘git log --follow’ failed on directories
in some older git versions?  Wouldn't it be safer to leave the code
that checks for directories and doesn't add ‘--follow’ for this case?

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: schwab@linux-m68k.org,  19045@debbugs.gnu.org
> Date: Mon, 12 Feb 2018 23:50:55 +0200
> 
> > Even if it's true, it means --follow doesn't do any harm when invoked
> > on directories, and we shouldn't disallow that.
> 
> I don't know the reason why disallowing directories was implemented
> in the first place.  Maybe ‘git log --follow’ failed on directories
> in some older git versions?  Wouldn't it be safer to leave the code
> that checks for directories and doesn't add ‘--follow’ for this case?

Sorry, I was confused and thought that you also added the
non-directory restriction.  I agree that we should not modify the code
that checks for directories, certainly not on the release branch.

Thanks.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

>> "Running %s in foreground..."
>
> Same as with "background".
>
>> "Running %s...OK = %d"
>
> Success (status 0): ...
>
>> "Running %s...FAILED (%s)"
>
> Failed  (status 3): ...
>
> Something like that. If the exit code is longer than 1 digit (pretty rare),
> maybe remove the extra space after "Failed".

Like this?

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index b0d2221..c1de2f0 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -290,16 +290,12 @@ vc-do-command
   (let* ((files
 	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
 		  (if (listp file-or-list) file-or-list (list file-or-list))))
+	 (message-truncate-lines t)
 	 (full-command
-	  ;; What we're doing here is preparing a version of the command
-	  ;; for display in a debug-progress message.  If it's fewer than
-	  ;; 20 characters display the entire command (without trailing
-	  ;; newline).  Otherwise display the first 20 followed by an ellipsis.
 	  (concat (if (string= (substring command -1) "\n")
 		      (substring command 0 -1)
 		    command)
-		  " "
-		  (vc-delistify (mapcar (lambda (s) (if (> (length s) 20) (concat (substring s 0 2) "...")  s)) flags))
+		  " " (vc-delistify flags)
 		  " " (vc-delistify files))))
     (save-current-buffer
       (unless (or (eq buffer t)
@@ -324,7 +320,7 @@ vc-do-command
 		       (apply 'start-file-process command (current-buffer)
                               command squeezed))))
 		(when vc-command-messages
-		  (message "Running %s in background..." full-command))
+		  (message "Running  in background: %s" full-command))
                 ;; Get rid of the default message insertion, in case we don't
                 ;; set a sentinel explicitly.
 		(set-process-sentinel proc #'ignore)
@@ -332,10 +328,11 @@ vc-do-command
 		(setq status proc)
 		(when vc-command-messages
 		  (vc-run-delayed
-		   (message "Running %s in background... done" full-command))))
+		    (let ((message-truncate-lines t))
+		      (message "Finished in background: %s" full-command)))))
 	    ;; Run synchronously
 	    (when vc-command-messages
-	      (message "Running %s in foreground..." full-command))
+	      (message "Running in foreground: %s" full-command))
 	    (let ((buffer-undo-list t))
 	      (setq status (apply 'process-file command nil t nil squeezed)))
 	    (when (and (not (eq t okstatus))
@@ -345,13 +342,14 @@ vc-do-command
                 (pop-to-buffer (current-buffer))
                 (goto-char (point-min))
                 (shrink-window-if-larger-than-buffer))
-	      (error "Running %s...FAILED (%s)" full-command
-		     (if (integerp status) (format "status %d" status) status)))
+	      (error "Failed (%s): %s"
+                     (if (integerp status) (format "status %d" status) status)
+		     full-command))
 	    (when vc-command-messages
-	      (message "Running %s...OK = %d" full-command status))))
+	      (message "Success (%d): %s" status full-command))))
 	(vc-run-delayed
-	 (run-hook-with-args 'vc-post-command-functions
-                             command file-or-list flags))
+	  (run-hook-with-args 'vc-post-command-functions
+			      command file-or-list flags))
 	status))))
 
 (defun vc-do-async-command (buffer root command &rest args)

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Fri, 16 Feb 2018 00:07:20 +0200
> Cc: 19045@debbugs.gnu.org
> 
> @@ -290,16 +290,12 @@ vc-do-command
>    (let* ((files
>  	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
>  		  (if (listp file-or-list) file-or-list (list file-or-list))))
> +	 (message-truncate-lines t)

This should have a comment explaining why it is needed.  It should
also mention that messages in this function are formatted in a certain
way so as to put the important parts at the beginning, due to
potential truncation of long messages.

> -		  (message "Running %s in background..." full-command))
> +		  (message "Running  in background: %s" full-command))
                                   ^^
Extra blank.

> -		   (message "Running %s in background... done" full-command))))
> +		    (let ((message-truncate-lines t))
> +		      (message "Finished in background: %s" full-command)))))

I find the original more in line with what we do elsewhere in Emacs.
If the problem is that "done" could be truncated, then please use

  Done in background: %s

> -	      (error "Running %s...FAILED (%s)" full-command
> -		     (if (integerp status) (format "status %d" status) status)))
> +	      (error "Failed (%s): %s"
> +                     (if (integerp status) (format "status %d" status) status)
> +		     full-command))
>  	    (when vc-command-messages
> -	      (message "Running %s...OK = %d" full-command status))))
> +	      (message "Success (%d): %s" status full-command))))

"Failed" and "Success" don't go together well, as they use different
grammatical form.  How about using "Done" here as well?  And won't
status always be zero when it succeeds?  If not, I'd use

  Done (status=%d): %s

Thanks.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Dmitry Gutov]

On 2/16/18 11:00 AM, Eli Zaretskii wrote:

>> -		  (message "Running %s in background..." full-command))
>> +		  (message "Running  in background: %s" full-command))
>                                     ^^
> Extra blank.

As previously discussed, it helps the full-command contents line up in 
the Messages buffer. "Running " is the same width as "Finished".

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> Cc: 19045@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 16 Feb 2018 12:20:21 +0200
> 
> On 2/16/18 11:00 AM, Eli Zaretskii wrote:
> 
> >> -		  (message "Running %s in background..." full-command))
> >> +		  (message "Running  in background: %s" full-command))
> >                                     ^^
> > Extra blank.
> 
> As previously discussed, it helps the full-command contents line up in 
> the Messages buffer. "Running " is the same width as "Finished".

Then please mention that in the comments to the code,as this surely
looks like a typo.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > As previously discussed, it helps the full-command contents line up in 
  > the Messages buffer. "Running " is the same width as "Finished".

If that's the reason for the apparently erroneous code, 
please add a comment to say so.

-- 
Dr Richard Stallman
President, Free Software Foundation (https://gnu.org, https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)
Skype: No way! See https://stallman.org/skype.html.

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 1467 bytes --]

>> -		  (message "Running %s in background..." full-command))
>> +		  (message "Running  in background: %s" full-command))
>                                    ^^
> Extra blank.
>
>> -		   (message "Running %s in background... done" full-command))))
>> +		    (let ((message-truncate-lines t))
>> +		      (message "Finished in background: %s" full-command)))))
>
> I find the original more in line with what we do elsewhere in Emacs.
> If the problem is that "done" could be truncated, then please use
>
>   Done in background: %s

Then extra blank is not needed when using “Done in background” because it
would require too many blanks to align with “Finished in background”.

>> -	      (error "Running %s...FAILED (%s)" full-command
>> -		     (if (integerp status) (format "status %d" status) status)))
>> +	      (error "Failed (%s): %s"
>> +                     (if (integerp status) (format "status %d" status) status)
>> +		     full-command))
>>  	    (when vc-command-messages
>> -	      (message "Running %s...OK = %d" full-command status))))
>> +	      (message "Success (%d): %s" status full-command))))
>
> "Failed" and "Success" don't go together well, as they use different
> grammatical form.  How about using "Done" here as well?  And won't
> status always be zero when it succeeds?  If not, I'd use

I see that the status in case of success is 1.

>   Done (status=%d): %s

Done in the following patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vc-dispatcher.patch --]
[-- Type: text/x-diff, Size: 3191 bytes --]

diff --git a/lisp/vc/vc-dispatcher.el b/lisp/vc/vc-dispatcher.el
index b0d2221..da9d346 100644
--- a/lisp/vc/vc-dispatcher.el
+++ b/lisp/vc/vc-dispatcher.el
@@ -290,16 +290,16 @@ vc-do-command
   (let* ((files
 	  (mapcar (lambda (f) (file-relative-name (expand-file-name f)))
 		  (if (listp file-or-list) file-or-list (list file-or-list))))
+	 ;; Keep entire commands in *Messages* but avoid resizing the
+	 ;; echo area.  Messages in this function are formatted in
+	 ;; a such way that the important parts are at the beginning,
+	 ;; due to potential truncation of long messages.
+	 (message-truncate-lines t)
 	 (full-command
-	  ;; What we're doing here is preparing a version of the command
-	  ;; for display in a debug-progress message.  If it's fewer than
-	  ;; 20 characters display the entire command (without trailing
-	  ;; newline).  Otherwise display the first 20 followed by an ellipsis.
 	  (concat (if (string= (substring command -1) "\n")
 		      (substring command 0 -1)
 		    command)
-		  " "
-		  (vc-delistify (mapcar (lambda (s) (if (> (length s) 20) (concat (substring s 0 2) "...")  s)) flags))
+		  " " (vc-delistify flags)
 		  " " (vc-delistify files))))
     (save-current-buffer
       (unless (or (eq buffer t)
@@ -324,7 +324,7 @@ vc-do-command
 		       (apply 'start-file-process command (current-buffer)
                               command squeezed))))
 		(when vc-command-messages
-		  (message "Running %s in background..." full-command))
+		  (message "Running in background: %s" full-command))
                 ;; Get rid of the default message insertion, in case we don't
                 ;; set a sentinel explicitly.
 		(set-process-sentinel proc #'ignore)
@@ -332,10 +332,11 @@ vc-do-command
 		(setq status proc)
 		(when vc-command-messages
 		  (vc-run-delayed
-		   (message "Running %s in background... done" full-command))))
+		    (let ((message-truncate-lines t))
+		      (message "Done in background: %s" full-command)))))
 	    ;; Run synchronously
 	    (when vc-command-messages
-	      (message "Running %s in foreground..." full-command))
+	      (message "Running in foreground: %s" full-command))
 	    (let ((buffer-undo-list t))
 	      (setq status (apply 'process-file command nil t nil squeezed)))
 	    (when (and (not (eq t okstatus))
@@ -345,13 +346,14 @@ vc-do-command
                 (pop-to-buffer (current-buffer))
                 (goto-char (point-min))
                 (shrink-window-if-larger-than-buffer))
-	      (error "Running %s...FAILED (%s)" full-command
-		     (if (integerp status) (format "status %d" status) status)))
+	      (error "Failed (%s): %s"
+		     (if (integerp status) (format "status %d" status) status)
+		     full-command))
 	    (when vc-command-messages
-	      (message "Running %s...OK = %d" full-command status))))
+	      (message "Done (status=%d): %s" status full-command))))
 	(vc-run-delayed
-	 (run-hook-with-args 'vc-post-command-functions
-                             command file-or-list flags))
+	  (run-hook-with-args 'vc-post-command-functions
+			      command file-or-list flags))
 	status))))
 
 (defun vc-do-async-command (buffer root command &rest args)

bug#19045: 25.0.50; [PATCH] vc-git-print-log use --follow

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: dgutov@yandex.ru,  19045@debbugs.gnu.org
> Date: Tue, 20 Feb 2018 22:39:25 +0200
> 
> Done in the following patch:

LGTM, thanks.