[PATCH 1/2] gitk: align the commit summary format to the documentation

[PATCH 1/2] gitk: align the commit summary format to the documentation

[Beat Bolli]

In 175d38c (SubmittingPatches: document how to reference previous commits,
2016-07-28) the format for referring to older commits was specified.

Make the text generated by the "Copy commit summary" command match this
format.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
Cc: Paul Mackerras <paulus@samba.org>
---
 gitk-git/gitk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 805a1c7..a27bf99 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -9382,7 +9382,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.7.2

[PATCH 2/2] SubmittingPatches: hint at gitk's "Copy commit summary" command

[Beat Bolli]

Amend the section on referencing previous commits with a hint to the
gitk command that was added exactly for this purpose.

Signed-off-by: Beat Bolli <dev+git@drbeat.li>
---
 Documentation/SubmittingPatches | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index 500230c..94a1661 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -124,7 +124,8 @@ archive, summarize the relevant points of the discussion.
 If you want to reference a previous commit in the history of a stable
 branch use the format "abbreviated sha1 (subject, date)". So for example
 like this: "Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
-noticed [...]".
+noticed [...]". The "Copy commit summary" command of gitk generates this
+format.
 
 
 (3) Generate your patch using Git tools out of your commits.
-- 
2.7.2

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Junio C Hamano]

Beat Bolli <dev+git@drbeat.li> writes:

> In 175d38c (SubmittingPatches: document how to reference previous commits,
> 2016-07-28) the format for referring to older commits was specified.
>
> Make the text generated by the "Copy commit summary" command match this
> format.

Hmph.  I didn't know gitk already had its own command to produce a
short string.  I actually think what it produces

> In 175d38c ("SubmittingPatches: document how to reference previous commits",
> 2016-07-28) the format for referring to older commits was specified.

is easier to read when pasted into a sentence than what the recent
update 175d38ca ("SubmittingPatches: document how to reference
previous commits", 2016-07-28) suggests to do, i.e.

> In 175d38c (SubmittingPatches: document how to reference previous commits,
> 2016-07-28) the format for referring to older commits was specified.

Heiko, Stefan, I think you two were involved in adding that new
paragraph.   What do you think?

Re: [PATCH 2/2] SubmittingPatches: hint at gitk's "Copy commit summary" command

[Junio C Hamano]

Beat Bolli <dev+git@drbeat.li> writes:

> @@ -124,7 +124,8 @@ archive, summarize the relevant points of the discussion.
>  If you want to reference a previous commit in the history of a stable
>  branch use the format "abbreviated sha1 (subject, date)". So for example
>  like this: "Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
> -noticed [...]".
> +noticed [...]". The "Copy commit summary" command of gitk generates this
> +format.

(continuing from my 1/2 review)  And if people agree that the format
gitk already uses is better, this text should probably read more
like:

    If you want to reference a previous commit in the history of a
    stable branch, use the format "abbreviated sha1 (subject, date)",
    with the subject enclosed in a pair of double-quotes, like this:

        Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
        noticed that ...

    The "Copy commit summary" command of gitk can be used to obtain this
    format.

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Stefan Beller]

On Fri, Aug 26, 2016 at 11:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Beat Bolli <dev+git@drbeat.li> writes:
>
>> In 175d38c (SubmittingPatches: document how to reference previous commits,
>> 2016-07-28) the format for referring to older commits was specified.
>>
>> Make the text generated by the "Copy commit summary" command match this
>> format.
>
> Hmph.  I didn't know gitk already had its own command to produce a
> short string.  I actually think what it produces

It was added in d835dbb91fe (gitk: Add a "Copy commit summary" command,
2015-07-18), it doesn't seem to be in your tree yet, so maybe wait
with this patch
until you pulled gitk?

>
>> In 175d38c ("SubmittingPatches: document how to reference previous commits",
>> 2016-07-28) the format for referring to older commits was specified.
>
> is easier to read when pasted into a sentence than what the recent
> update 175d38ca ("SubmittingPatches: document how to reference
> previous commits", 2016-07-28) suggests to do, i.e.
>
>> In 175d38c (SubmittingPatches: document how to reference previous commits,
>> 2016-07-28) the format for referring to older commits was specified.
>
> Heiko, Stefan, I think you two were involved in adding that new
> paragraph.   What do you think?

So the subtle difference is adding '"' around the commit message subject?

I agree we should fix that.

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Beat Bolli]

On 26.08.16 21:16, Stefan Beller wrote:
> On Fri, Aug 26, 2016 at 11:24 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Beat Bolli <dev+git@drbeat.li> writes:
>>
>>> In 175d38c (SubmittingPatches: document how to reference previous commits,
>>> 2016-07-28) the format for referring to older commits was specified.
>>>
>>> Make the text generated by the "Copy commit summary" command match this
>>> format.
>>
>> Hmph.  I didn't know gitk already had its own command to produce a
>> short string.  I actually think what it produces
> 
> It was added in d835dbb91fe (gitk: Add a "Copy commit summary" command,
> 2015-07-18), it doesn't seem to be in your tree yet, so maybe wait
> with this patch
> until you pulled gitk?

This commit was part of release 2.6.0.

>>> In 175d38c ("SubmittingPatches: document how to reference previous commits",
>>> 2016-07-28) the format for referring to older commits was specified.
>>
>> is easier to read when pasted into a sentence than what the recent
>> update 175d38ca ("SubmittingPatches: document how to reference
>> previous commits", 2016-07-28) suggests to do, i.e.
>>
>>> In 175d38c (SubmittingPatches: document how to reference previous commits,
>>> 2016-07-28) the format for referring to older commits was specified.
>>
>> Heiko, Stefan, I think you two were involved in adding that new
>> paragraph.   What do you think?
> 
> So the subtle difference is adding '"' around the commit message subject?
> 
> I agree we should fix that.

So would you prepare a amendment to your documentation commit so that
Junio can disregard my two patches?

Thanks,
Beat

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Junio C Hamano]

Beat Bolli <dev+git@drbeat.li> writes:

> On 26.08.16 21:16, Stefan Beller wrote:
>> I agree we should fix that.
>
> So would you prepare a amendment to your documentation commit so that
> Junio can disregard my two patches?

I think the mention of gitk having a feature to easily give you a
commit name in the preferred format added by your 2/2 is worth
keeping.  I am not Stefan, though ;-)

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Stefan Beller]

On Fri, Aug 26, 2016 at 2:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Beat Bolli <dev+git@drbeat.li> writes:
>
>> On 26.08.16 21:16, Stefan Beller wrote:
>>> I agree we should fix that.
>>
>> So would you prepare a amendment to your documentation commit so that
>> Junio can disregard my two patches?
>
> I think the mention of gitk having a feature to easily give you a
> commit name in the preferred format added by your 2/2 is worth
> keeping.  I am not Stefan, though ;-)

I just sent a patch to the list to fix the formatting in SubmittingPatches.
Please keep 2/2, though ?

Thanks,
Stefan

Re: [PATCH 2/2] SubmittingPatches: hint at gitk's "Copy commit summary" command

[Jacob Keller]

On Fri, Aug 26, 2016 at 11:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Beat Bolli <dev+git@drbeat.li> writes:
>
>> @@ -124,7 +124,8 @@ archive, summarize the relevant points of the discussion.
>>  If you want to reference a previous commit in the history of a stable
>>  branch use the format "abbreviated sha1 (subject, date)". So for example
>>  like this: "Commit f86a374 (pack-bitmap.c: fix a memleak, 2015-03-30)
>> -noticed [...]".
>> +noticed [...]". The "Copy commit summary" command of gitk generates this
>> +format.
>
> (continuing from my 1/2 review)  And if people agree that the format
> gitk already uses is better, this text should probably read more
> like:
>
>     If you want to reference a previous commit in the history of a
>     stable branch, use the format "abbreviated sha1 (subject, date)",
>     with the subject enclosed in a pair of double-quotes, like this:
>
>         Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
>         noticed that ...
>
>     The "Copy commit summary" command of gitk can be used to obtain this
>     format.


Tangent, but I was wondering if this would make a good built-in
format, but then I looked and realized that the built-in formats
didn't make much sense to me... I'm not sure where the actual format
gets displayed so I am wondering if we had any thoughts about adding
some --pretty=summary that we could add? I know it can be implemented
via a user defined format but thought it might be worth building it in
since it's a pretty common use case?

Thanks,
Jake

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Johannes Sixt]

Am 26.08.2016 um 20:24 schrieb Junio C Hamano:
> Beat Bolli <dev+git@drbeat.li> writes:
>> In 175d38c ("SubmittingPatches: document how to reference previous commits",
>> 2016-07-28) the format for referring to older commits was specified.
>
> is easier to read when pasted into a sentence than what the recent
> update 175d38ca ("SubmittingPatches: document how to reference
> previous commits", 2016-07-28) suggests to do, i.e.

While it may be easier to read due to the extra mark-up, the resulting 
text where such a quotation appears does not flow well, IMO. A commit 
message text that references another commit reads more fluently without 
the quotes around the summary line because the quoted text is not so 
much a quotation that must be marked, but a parenthetical statement.

I absolutely welcome the proposed change to gitk, because I always edit 
out the double-quotes.

-- Hannes

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 26.08.2016 um 20:24 schrieb Junio C Hamano:
>> Beat Bolli <dev+git@drbeat.li> writes:
>>> In 175d38c ("SubmittingPatches: document how to reference previous commits",
>>> 2016-07-28) the format for referring to older commits was specified.
>>
>> is easier to read when pasted into a sentence than what the recent
>> update 175d38ca ("SubmittingPatches: document how to reference
>> previous commits", 2016-07-28) suggests to do, i.e.
>
> While it may be easier to read due to the extra mark-up, the resulting
> text where such a quotation appears does not flow well, IMO. A commit
> message text that references another commit reads more fluently
> without the quotes around the summary line because the quoted text is
> not so much a quotation that must be marked, but a parenthetical
> statement.
>
> I absolutely welcome the proposed change to gitk, because I always
> edit out the double-quotes.

I think that is highly subjective, and as you very well may know,
I've been referring to commits without double-quote pair, and have
an obvious bias for something I am used to ;-)

I do not see the "" as introducing a quotation.  I just view it as
very similar to the "" in the following sentence:

    The commit whose title is "foo bar" did not consider there is
    also need to consider baz.

The whole thing is inside () pair, so I agree that with or without
"" pair, it is possible to see where the title ends.  So I do not
have a strong opinion either way.

Re: [PATCH 1/2] gitk: align the commit summary format to the documentation

[Jeff King]

On Mon, Aug 29, 2016 at 11:17:19AM -0700, Junio C Hamano wrote:

> > While it may be easier to read due to the extra mark-up, the resulting
> > text where such a quotation appears does not flow well, IMO. A commit
> > message text that references another commit reads more fluently
> > without the quotes around the summary line because the quoted text is
> > not so much a quotation that must be marked, but a parenthetical
> > statement.
> >
> > I absolutely welcome the proposed change to gitk, because I always
> > edit out the double-quotes.
> 
> I think that is highly subjective, and as you very well may know,
> I've been referring to commits without double-quote pair, and have
> an obvious bias for something I am used to ;-)
> 
> I do not see the "" as introducing a quotation.  I just view it as
> very similar to the "" in the following sentence:
> 
>     The commit whose title is "foo bar" did not consider there is
>     also need to consider baz.
> 
> The whole thing is inside () pair, so I agree that with or without
> "" pair, it is possible to see where the title ends.  So I do not
> have a strong opinion either way.

I have an alias which produces similar output, without the double-quotes
(probably because I stole it from you originally).

I have noticed over the years that the output is occasionally ugly when
the commit names have parentheses themselves. E.g.:

  $ git config alias.ll
  !git --no-pager log -1 --pretty='tformat:%h (%s, %ad)' --date=short

  $ git ll 7e97e10
  7e97e10 (die(_("BUG")): avoid translating bug messages, 2016-07-26)

  $ git ll fa90ab4
  fa90ab4 (t3404: fix a grammo (commands are ran -> commands are run), 2016-06-29)

Adding quotes can help with that. OTOH, I think it just introduces the
same problem with a different character. E.g.:

  $ git ll be00b57
  be00b57 (provide an initializer for "struct object_info", 2016-08-11)

  $ git llq be00b57
  be00b57 ("provide an initializer for "struct object_info"", 2016-08-11)

Perhaps one could write a script to find a custom pretty non-conflicting
delimiter for each case, but I don't know if it's worth the effort. :)

-Peff