diff mbox series

[v2,1/2] gitk: drop quotes in copysummary format

Message ID 75dc131f0575cbe5d885af8897cbde054f7c07cf.1576197846.git.liu.denton@gmail.com (mailing list archive)
State New, archived
Headers show
Series gitk: match Git's 'reference' pretty format | expand

Commit Message

Denton Liu Dec. 13, 2019, 12:44 a.m. UTC
In an earlier commit[1], git learned the 'reference' pretty format. This
format was very similar to the format that copysummary used but it omits
the quotes surrounding the commit's subject. Remove the quotes from the
format in copysummary in order to match the 'reference' pretty format.

It seems the consensus is that the unquoted form is used more often
anyway[2] so this change should be acceptable.

[These commits are in git.git.]
[1]: 1f0fc1db85 (pretty: implement 'reference' format, 2019-11-19)
[2]: fb2ffa77a6 (SubmittingPatches: remove dq from commit reference, 2019-11-19)

Signed-off-by: Denton Liu <liu.denton@gmail.com>
---
 gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Dec. 13, 2019, 5:12 p.m. UTC | #1
Denton Liu <liu.denton@gmail.com> writes:

> In an earlier commit[1], git learned the 'reference' pretty format. This
> format was very similar to the format that copysummary used but it omits
> the quotes surrounding the commit's subject. Remove the quotes from the
> format in copysummary in order to match the 'reference' pretty format.
>
> It seems the consensus is that the unquoted form is used more often
> anyway[2] so this change should be acceptable.
>
> [These commits are in git.git.]
> [1]: 1f0fc1db85 (pretty: implement 'reference' format, 2019-11-19)
> [2]: fb2ffa77a6 (SubmittingPatches: remove dq from commit reference, 2019-11-19)

This change to gitk actually can easily be controversial.

The "used more often" census IIUC was done on _this_ project, and
SubmittingPatches is merely a guideline to contribute to _this_
project, but the audience of gitk that wants to use the copysummary
format is a lot wider than that.

This patch alone may not be worth sinking the time on for that
reason.

Having said that, if we teach --pretty=reference to optionally use a
custom format via a configuration variable, it would make quite a
lot of sense to do an update _like_ this patch does to gitk.  It
would probably allow the code to read from the same configuration
variable, instead of replacing a hardcoded format string with
another hardcoded format string, though.

Thanks.

> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  gitk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitk b/gitk
> index abe4805ade..d07e3302de 100755
> --- a/gitk
> +++ b/gitk
> @@ -9429,7 +9429,7 @@ proc mktaggo {} {
>  proc copysummary {} {
>      global rowmenuid autosellen
>  
> -    set format "%h (\"%s\", %ad)"
> +    set format "%h (%s, %ad)"
>      set cmd [list git show -s --pretty=format:$format --date=short]
>      if {$autosellen < 40} {
>          lappend cmd --abbrev=$autosellen
Paul Mackerras Dec. 15, 2019, 4:19 a.m. UTC | #2
On Thu, Dec 12, 2019 at 04:44:49PM -0800, Denton Liu wrote:
> In an earlier commit[1], git learned the 'reference' pretty format. This
> format was very similar to the format that copysummary used but it omits
> the quotes surrounding the commit's subject. Remove the quotes from the
> format in copysummary in order to match the 'reference' pretty format.
> 
> It seems the consensus is that the unquoted form is used more often
> anyway[2] so this change should be acceptable.

I don't see any discussion about removing the quotes in the Linux
kernel community, and the Documentation/process/submitting-patches.rst
in the kernel tree says (or at least implies) that the quotes are
required.  So I am not convinced this change is a good thing to do.

In fact the gitk commit summary/reference format doesn't match what
the kernel wants exactly, in that gitk puts in the author-date, and
submitting-patches.rst doesn't have it.  There are maintainers that
have scripts that automatically check the format of Fixes: lines in
submaintainers' trees and send email if they detect one with the wrong
formatting.  Thus I would find it more convenient to change the format
to %h (\"s\") than to "%h (%s, %ad)".

Maybe the solution is to make the format customizable.

> [These commits are in git.git.]
> [1]: 1f0fc1db85 (pretty: implement 'reference' format, 2019-11-19)
> [2]: fb2ffa77a6 (SubmittingPatches: remove dq from commit reference, 2019-11-19)
> 
> Signed-off-by: Denton Liu <liu.denton@gmail.com>
> ---
>  gitk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gitk b/gitk
> index abe4805ade..d07e3302de 100755
> --- a/gitk
> +++ b/gitk
> @@ -9429,7 +9429,7 @@ proc mktaggo {} {
>  proc copysummary {} {
>      global rowmenuid autosellen
>  
> -    set format "%h (\"%s\", %ad)"
> +    set format "%h (%s, %ad)"
>      set cmd [list git show -s --pretty=format:$format --date=short]
>      if {$autosellen < 40} {
>          lappend cmd --abbrev=$autosellen
> -- 
> 2.24.1.664.g198078bb5a

Paul.
diff mbox series

Patch

diff --git a/gitk b/gitk
index abe4805ade..d07e3302de 100755
--- a/gitk
+++ b/gitk
@@ -9429,7 +9429,7 @@  proc mktaggo {} {
 proc copysummary {} {
     global rowmenuid autosellen
 
-    set format "%h (\"%s\", %ad)"
+    set format "%h (%s, %ad)"
     set cmd [list git show -s --pretty=format:$format --date=short]
     if {$autosellen < 40} {
         lappend cmd --abbrev=$autosellen