{"thread":{"id":"39867","subject":"[PATCH v3] gitk: Add a \"Copy commit summary\" command","startedAt":"2015-07-17T08:39:03Z","lastAt":"2015-07-17T20:59:17Z","messageCount":4,"participants":["Beat Bolli","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"266225","messageId":"205a18f7f8a7892a1fa63a91a84bfd9b@drbeat.li","threadId":"39867","inReplyTo":null,"subject":"[PATCH v3] gitk: Add a \"Copy commit summary\" command","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2015-07-17T08:39:03Z","receivedAt":"2015-07-17T08:39:03Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"When referring to earlier commits in commit messages or other text, one\nof the established formats is\n\n (\"\", )\n\nAdd a \"Copy commit summary\" command to the context menu that puts this\ntext for the currently selected commit on the clipboard. This makes it\neasy for our users to create well-formatted commit references.\n\nThe is produced with the %h format specifier to make it\nunique. Its minimum length can be controlled with the config setting\ncore.abbrev.\n\nSigned-off-by: Beat Bolli \nReviewed-by: Eric Sunshine \nReviewed-by: Johannes Sixt \nCc: Paul Mackerras \n\n---\nChanges since v2:\n- call git log to produce a unique \n- use the short date format\n\nChanges since v1:\n- drop the \"commit \" literal in front of the \n---\n gitk-git/gitk | 12 ++++++++++++\n 1 file changed, 12 insertions(+)\n\ndiff --git a/gitk-git/gitk b/gitk-git/gitk\nindex 9a2daf3..4915f53 100755\n--- a/gitk-git/gitk\n+++ b/gitk-git/gitk\n@@ -2617,6 +2617,7 @@ proc makewindow {} {\n \t{mc \"Diff selected -> this\" command {diffvssel 1}}\n \t{mc \"Make patch\" command mkpatch}\n \t{mc \"Create tag\" command mktag}\n+\t{mc \"Copy commit summary\" command copysummary}\n \t{mc \"Write commit to file\" command writecommit}\n \t{mc \"Create new branch\" command mkbranch}\n \t{mc \"Cherry-pick this commit\" command cherrypick}\n@@ -9341,6 +9342,17 @@ proc mktaggo {} {\n mktagcan\n }\n\n+proc copysummary {} {\n+ global rowmenuid\n+\n+ set format \"%h (\\\"%s\\\", %ad)\"\n+ set summary [exec git show -s --pretty=format:$format --date=short \n\\\n+ $rowmenuid]\n+\n+ clipboard clear\n+ clipboard append $summary\n+}\n+\n proc writecommit {} {\n global rowmenuid wrcomtop commitinfo wrcomcmd NS\n\n-- \n2.4.0\n"},{"id":"266255","messageId":"CAPig+cR=u_ak_=J=gSAWfiNB01R7FBG+bCrx+k1HNAE0xHtwFQ@mail.gmail.com","threadId":"39867","inReplyTo":"205a18f7f8a7892a1fa63a91a84bfd9b@drbeat.li","subject":"Re: [PATCH v3] gitk: Add a \"Copy commit summary\" command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-07-17T17:17:33Z","receivedAt":"2015-07-17T17:17:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Jul 17, 2015 at 4:39 AM, Beat Bolli wrote:\n> When referring to earlier commits in commit messages or other text, one\n> of the established formats is\n>\n> (\"\", )\n>\n> Add a \"Copy commit summary\" command to the context menu that puts this\n> text for the currently selected commit on the clipboard. This makes it\n> easy for our users to create well-formatted commit references.\n>\n> The is produced with the %h format specifier to make it\n> unique. Its minimum length can be controlled with the config setting\n> core.abbrev.\n>\n> Signed-off-by: Beat Bolli \n> Reviewed-by: Eric Sunshine \n> Reviewed-by: Johannes Sixt \n\nYou should drop these Reviewed-by: footers, as they imply that the\ncode was thoroughly digested and the implementation deemed correct.\nHannes at least gave advice about abbreviation length and may deserve\nmention via a Helped-by: footer (if you take his advice and feel his\ncontribution worthy); however, in my case, I merely made a very minor\nobservation about the output format, not even worth a Helped-by: (and\ncertainly not a Reviewed-by:).\n\n> Cc: Paul Mackerras \n>\n> ---\n> Changes since v2:\n> - call git log to produce a unique \n> - use the short date format\n>\n> Changes since v1:\n> - drop the \"commit \" literal in front of the \n> ---\n> gitk-git/gitk | 12 ++++++++++++\n> 1 file changed, 12 insertions(+)\n>\n> diff --git a/gitk-git/gitk b/gitk-git/gitk\n> index 9a2daf3..4915f53 100755\n> --- a/gitk-git/gitk\n> +++ b/gitk-git/gitk\n> @@ -2617,6 +2617,7 @@ proc makewindow {} {\n> {mc \"Diff selected -> this\" command {diffvssel 1}}\n> {mc \"Make patch\" command mkpatch}\n> {mc \"Create tag\" command mktag}\n> + {mc \"Copy commit summary\" command copysummary}\n> {mc \"Write commit to file\" command writecommit}\n> {mc \"Create new branch\" command mkbranch}\n> {mc \"Cherry-pick this commit\" command cherrypick}\n> @@ -9341,6 +9342,17 @@ proc mktaggo {} {\n> mktagcan\n> }\n>\n> +proc copysummary {} {\n> + global rowmenuid\n> +\n> + set format \"%h (\\\"%s\\\", %ad)\"\n> + set summary [exec git show -s --pretty=format:$format --date=short \\\n> + $rowmenuid]\n> +\n> + clipboard clear\n> + clipboard append $summary\n> +}\n> +\n> proc writecommit {} {\n> global rowmenuid wrcomtop commitinfo wrcomcmd NS\n>\n> --\n> 2.4.0\n"},{"id":"266256","messageId":"xmqqwpxy1yoo.fsf@gitster.dls.corp.google.com","threadId":"39867","inReplyTo":"CAPig+cR=u_ak_=J=gSAWfiNB01R7FBG+bCrx+k1HNAE0xHtwFQ@mail.gmail.com","subject":"Re: [PATCH v3] gitk: Add a \"Copy commit summary\" command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-07-17T17:28:07Z","receivedAt":"2015-07-17T17:28:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n>> Signed-off-by: Beat Bolli \n>> Reviewed-by: Eric Sunshine \n>> Reviewed-by: Johannes Sixt \n>\n> You should drop these Reviewed-by: footers, as they imply that the\n> code was thoroughly digested and the implementation deemed correct.\n\n... and the most importantly, the named people said that themselves.\n\nI do not think that happened here (yet).\n\n>> +proc copysummary {} {\n>> + global rowmenuid\n>> +\n>> + set format \"%h (\\\"%s\\\", %ad)\"\n>> + set summary [exec git show -s --pretty=format:$format --date=short \\\n>> + $rowmenuid]\n>> +\n>> + clipboard clear\n>> + clipboard append $summary\n>> +}\n>> +\n\nI think this is a reasonable implementation. The usual \"spawning a\nprocess for each commit is too expensive\" would not apply, because\nit is done on demand only for the single commit that the end-user\nspecified.\n"},{"id":"266266","messageId":"55A96CA5.3050504@drbeat.li","threadId":"39867","inReplyTo":"xmqqwpxy1yoo.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH v3] gitk: Add a \"Copy commit summary\" command","fromName":"Beat Bolli","fromEmail":"dev+git@drbeat.li","sentAt":"2015-07-17T20:59:17Z","receivedAt":"2015-07-17T20:59:17Z","isPatch":true,"sender":{"key":"dev+git@drbeat.li","avatar":"https://avatars.githubusercontent.com/u/21444?v=4"},"body":"On 17.07.15 19:28, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> \n>>> Signed-off-by: Beat Bolli \n>>> Reviewed-by: Eric Sunshine \n>>> Reviewed-by: Johannes Sixt \n>>\n>> You should drop these Reviewed-by: footers, as they imply that the\n>> code was thoroughly digested and the implementation deemed correct.\n> \n> ... and the most importantly, the named people said that themselves.\n> \n> I do not think that happened here (yet).\n> \n>>> +proc copysummary {} {\n>>> + global rowmenuid\n>>> +\n>>> + set format \"%h (\\\"%s\\\", %ad)\"\n>>> + set summary [exec git show -s --pretty=format:$format --date=short \\\n>>> + $rowmenuid]\n>>> +\n>>> + clipboard clear\n>>> + clipboard append $summary\n>>> +}\n>>> +\n> \n> I think this is a reasonable implementation. The usual \"spawning a\n> process for each commit is too expensive\" would not apply, because\n> it is done on demand only for the single commit that the end-user\n> specified.\n\nThanks, Junio! That was my thought as well.\n\nSo, the question remains now if adding something like\n--abbrev=$autosellen (maybe only if it's not set to its default value),\nas Paul suggested, would make sense.\n\nBeat\n"}]}