GNU bug report logs - #59414
29.0.50; Have vc-git-expanded-log-entry pass --stat

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 59414 in the body.
You can then email your comments to 59414 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 20 Nov 2022 10:35:16 -0700

Hello,

I would like to add --stat to the list of options passed to git-log(1)
by vc-git-expanded-log-entry.  I think it makes VC root logs for Git
repositories much more informative with little disadvantage.  Any comments?

-- 
Sean Whitton

Message #8 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 20 Nov 2022 19:52:55 +0200

> I would like to add --stat to the list of options passed to git-log(1)
> by vc-git-expanded-log-entry.  I think it makes VC root logs for Git
> repositories much more informative with little disadvantage.  Any comments?

Not sure about adding --stat by default since often it produces too long
multi-line output.  But definitely the options should be customizable
to avoid adding such cruft to config files:

  (with-eval-after-load 'vc-git
    ;; OVERRIDDEN AND ADDED "--pretty=fuller"
    (defun vc-git-expanded-log-entry (revision)
      (with-temp-buffer
        (apply 'vc-git-command t nil nil (list "log" revision "-1" "--pretty=fuller" "--"))
        (goto-char (point-min))
        (unless (eobp)
          ;; Indent the expanded log entry.
          (while (re-search-forward "^  " nil t)
            (replace-match "")
            (forward-line))
          (buffer-string)))))

Message #11 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Juri Linkov <juri <at> linkov.net>, Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 20 Nov 2022 22:14:31 +0200

On 20.11.2022 19:52, Juri Linkov wrote:
>> I would like to add --stat to the list of options passed to git-log(1)
>> by vc-git-expanded-log-entry.  I think it makes VC root logs for Git
>> repositories much more informative with little disadvantage.  Any comments?
> Not sure about adding --stat by default since often it produces too long
> multi-line output.  But definitely the options should be customizable
> to avoid adding such cruft to config files:

Some new -switches variable could work. One should also ask themselves 
whether they want to apply the new switches to the "full" log as well -- 
one you get by pressing 'C-x v l'.

Message #14 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>
Cc: 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 20 Nov 2022 14:58:10 -0700

Hello,

On Sun 20 Nov 2022 at 07:52PM +02, Juri Linkov wrote:

>> I would like to add --stat to the list of options passed to git-log(1)
>> by vc-git-expanded-log-entry.  I think it makes VC root logs for Git
>> repositories much more informative with little disadvantage.  Any comments?
>
> Not sure about adding --stat by default since often it produces too long
> multi-line output.  But definitely the options should be customizable
> to avoid adding such cruft to config files:

We already have vc-git-log-switches.  git-log(1) gets called *without*
it in (at least) the following places:

- vc-git-log-outgoing
- vc-git-log-incoming
- vc-git-log-search
- vc-git-expanded-log-entry
- vc-git-region-history

I guess that the first three should probably use vc-git-log-switches if
anything?  And so we would want a separate option for
vc-git-expanded-log-entry.  Not sure about vc-git-region-history.

-- 
Sean Whitton

Message #17 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, Juri Linkov <juri <at> linkov.net>
Cc: 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 21 Nov 2022 00:11:57 +0200

On 20.11.2022 23:58, Sean Whitton wrote:
> And so we would want a separate option for
> vc-git-expanded-log-entry.

Do we anticipate people using different switches for it from the first 
three? Like, do you want to add --stat only there and not in those other 
cases?

vc-git-region-history - no, probably not, since it comes with the diffs 
already.

Message #20 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 59414 <at> debbugs.gnu.org, Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 21 Nov 2022 13:29:07 -0700

Hello,

On Mon 21 Nov 2022 at 12:11AM +02, Dmitry Gutov wrote:

> On 20.11.2022 23:58, Sean Whitton wrote:
>> And so we would want a separate option for
>> vc-git-expanded-log-entry.
>
> Do we anticipate people using different switches for it from the first three?
> Like, do you want to add --stat only there and not in those other cases?

Yes: if you add --stat to vc-git-log-switches it simply breaks Log View.

-- 
Sean Whitton

Message #23 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>, Dmitry Gutov <dgutov <at> yandex.ru>,
 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sat, 03 Dec 2022 00:04:17 -0700

Hello,

On Sun 20 Nov 2022 at 02:58PM -07, Sean Whitton wrote:

> We already have vc-git-log-switches.  git-log(1) gets called *without*
> it in (at least) the following places:
>
> - vc-git-log-outgoing
> - vc-git-log-incoming
> - vc-git-log-search
> - vc-git-expanded-log-entry
> - vc-git-region-history
>
> I guess that the first three should probably use vc-git-log-switches if
> anything?  And so we would want a separate option for
> vc-git-expanded-log-entry.  Not sure about vc-git-region-history.

I think that we actually need two defcustoms for the regular logs and
shortlogs:

- vc-git-print-log -- should choose which defcustom to include based on
                      its SHORTLOG parameter
- vc-git-log-outgoing -- vc-git-shortlog-switches
- vc-git-log-incoming -- vc-git-shortlog-switches
- vc-git-log-search -- vc-git-log-switches
- vc-git-expanded-log-entry -- vc-git-log-switches

This is because some options are incompatible with shortlogs, such as
--stat.  So, the proposed change is to add the new defcustom, change
vc-git-print-log to use both defcustoms, and change all the other
functions to use one of them.  How does    this sound?

-- 
Sean Whitton

Message #26 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 59414 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 04 Dec 2022 21:19:51 +0200

> - vc-git-print-log -- should choose which defcustom to include based on
>                       its SHORTLOG parameter
> - vc-git-log-outgoing -- vc-git-shortlog-switches
> - vc-git-log-incoming -- vc-git-shortlog-switches
> - vc-git-log-search -- vc-git-log-switches
> - vc-git-expanded-log-entry -- vc-git-log-switches
>
> This is because some options are incompatible with shortlogs, such as
> --stat.  So, the proposed change is to add the new defcustom, change
> vc-git-print-log to use both defcustoms, and change all the other
> functions to use one of them.  How does    this sound?

Your classification correctly reflects the current situation
where two types of logs are in use by these commands.  So I think
two defcustoms as the minimal number of customization knobs
is a good idea.

Message #29 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Juri Linkov <juri <at> linkov.net>, Dmitry Gutov <dgutov <at> yandex.ru>,
 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 04 Dec 2022 15:57:52 -0700

[Message part 1 (text/plain, inline)]

Hello,

Thanks.  Here's a patch.

-- 
Sean Whitton

[0001-Improve-passing-user-switches-to-Git-log-commands.patch (text/x-patch, attachment)]

Message #32 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Sean Whitton <spwhitton <at> spwhitton.name>, Juri Linkov <juri <at> linkov.net>,
 59414 <at> debbugs.gnu.org
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 5 Dec 2022 02:54:04 +0200

On 05/12/2022 00:57, Sean Whitton wrote:
> Hello,
> 
> Thanks.  Here's a patch.

Thank you.

Looks good, please install.

Message #37 received at 59414-done <at> debbugs.gnu.org (full text, mbox):

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: Dmitry Gutov <dgutov <at> yandex.ru>, 59414-done <at> debbugs.gnu.org
Cc: Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Sun, 04 Dec 2022 22:09:01 -0700

Hello,

Thanks for looking it over.  Installed.

-- 
Sean Whitton

Message #40 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 59414 <at> debbugs.gnu.org, dgutov <at> yandex.ru, juri <at> linkov.net
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 05 Dec 2022 14:29:01 +0200

> From: Sean Whitton <spwhitton <at> spwhitton.name>
> Date: Sun, 04 Dec 2022 15:57:52 -0700
> 
> @@ -1340,16 +1348,16 @@ vc-git-print-log
>  
>  (defun vc-git-log-outgoing (buffer remote-location)
>    (vc-setup-buffer buffer)
> -  (vc-git-command
> -   buffer 'async nil
> -   "log"
> -   "--no-color" "--graph" "--decorate" "--date=short"
> -   (format "--pretty=tformat:%s" (car vc-git-root-log-format))
> -   "--abbrev-commit"
> -   (concat (if (string= remote-location "")
> -	       "@{upstream}"
> -	     remote-location)
> -	   "..HEAD")))
> +  (apply #'vc-git-command buffer 'async nil
> +         `("log"
> +           "--no-color" "--graph" "--decorate" "--date=short"
> +           ,(format "--pretty=tformat:%s" (car vc-git-root-log-format))
> +           "--abbrev-commit"
> +           ,@(ensure-list vc-git-shortlog-switches)
> +           ,(concat (if (string= remote-location "")
> +	                "@{upstream}"
> +	              remote-location)
> +	            "..HEAD"))))

Why the change from vc-git-command to 'apply'?  The former took care for
setting up the I/O encoding for the Git command, while the latter just uses
the process defaults, which are not necessarily right for the underlying
system and locale.

In general, I'd prefer that invocations of all the Git commands went through
a single function, so that we could make sure the encoding/decoding stuff,
which is entirely non-trivial with Git, is done correctly in a single place
that is easy to audit and maintain.  I know that not all the commands are
invoked through there, but making more of them do so is going in the
direction that is 180° opposite to what we should strive to.

Thanks.

Message #43 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>, Sean Whitton <spwhitton <at> spwhitton.name>
Cc: 59414 <at> debbugs.gnu.org, juri <at> linkov.net
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 5 Dec 2022 14:40:39 +0200

On 05/12/2022 14:29, Eli Zaretskii wrote:
> Why the change from vc-git-command to 'apply'?  The former took care for
> setting up the I/O encoding for the Git command, while the latter just uses
> the process defaults, which are not necessarily right for the underlying
> system and locale.
> 
> In general, I'd prefer that invocations of all the Git commands went through
> a single function, so that we could make sure the encoding/decoding stuff,
> which is entirely non-trivial with Git, is done correctly in a single place
> that is easy to audit and maintain.  I know that not all the commands are
> invoked through there, but making more of them do so is going in the
> direction that is 180° opposite to what we should strive to.

Both cases use 'vc-git-command', don't they?

'apply' is just about how the arguments are passed.

Message #46 received at 59414 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 59414 <at> debbugs.gnu.org, juri <at> linkov.net, spwhitton <at> spwhitton.name
Subject: Re: bug#59414: 29.0.50; Have vc-git-expanded-log-entry pass --stat
Date: Mon, 05 Dec 2022 14:57:53 +0200

> Date: Mon, 5 Dec 2022 14:40:39 +0200
> Cc: 59414 <at> debbugs.gnu.org, juri <at> linkov.net
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> 
> On 05/12/2022 14:29, Eli Zaretskii wrote:
> > Why the change from vc-git-command to 'apply'?  The former took care for
> > setting up the I/O encoding for the Git command, while the latter just uses
> > the process defaults, which are not necessarily right for the underlying
> > system and locale.
> > 
> > In general, I'd prefer that invocations of all the Git commands went through
> > a single function, so that we could make sure the encoding/decoding stuff,
> > which is entirely non-trivial with Git, is done correctly in a single place
> > that is easy to audit and maintain.  I know that not all the commands are
> > invoked through there, but making more of them do so is going in the
> > direction that is 180° opposite to what we should strive to.
> 
> Both cases use 'vc-git-command', don't they?
> 
> 'apply' is just about how the arguments are passed.

Sorry, too little coffee, I guess.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.