Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-18 Thread Beat Bolli
On 18.07.15 14:23, Paul Mackerras wrote:
 On Fri, Jul 17, 2015 at 08:30:24AM -0700, Junio C Hamano wrote:
 Paul Mackerras pau...@samba.org writes:

 We have an item in the preferences menu to control the SHA1 length
 that is automatically selected when going to a new commit.  It's
 stored in the variable $autosellen.  That seems like it would be a
 reasonable choice for the SHA1 length to use here.

 Reusing a configuration that is used to control something similar
 sounds sensible to me.

 The only possible
 problem is that it defaults to 40 and so might give an overly long
 result for some users.  Maybe you could use $autosellen but limit it
 to at most 12 or 16 or something like that.

 How is the thing that is automatically selected when going to a new
 commit used by the end user?  What is the reason why people may
 want to configure it?  I understand that this is the string that
 goes into the selection buffer, so presumably people are using this
 selection to paste elsewhere?  If so, that sounds like very similar
 to Beat's use case---perhaps if 40 is too long for Beat's use case
 as a sensible default, then it is also too long for its original use
 case?
 
 It's used for pasting into commit messages and emails, and it's used
 for pasting onto the command line when typing git commands.  For the
 second, the length doesn't matter; the limit was added for the first
 case.
 
 Or do you expect it to be common to want to use autosellen set to 40
 and Beat's abbrev len set to much shorter, e.g. 16?  If so they may
 deserve two different settings, with different defaults.
 
 I would think that if $autosellen is 40 it's almost certainly because
 the user hasn't found that control in the preferences window. :)
 
 Artificially limiting it to 12 or 16 does not sound all that
 sensible, though.
 
 Adding --abbrev=$autosellen if $autosellen is not 40 sounds like it
 would do what we want.

That's exactly what I did in v4 of the patch:
http://article.gmane.org/gmane.comp.version-control.git/274161

Thanks,
Beat
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-18 Thread Paul Mackerras
On Fri, Jul 17, 2015 at 08:30:24AM -0700, Junio C Hamano wrote:
 Paul Mackerras pau...@samba.org writes:
 
  We have an item in the preferences menu to control the SHA1 length
  that is automatically selected when going to a new commit.  It's
  stored in the variable $autosellen.  That seems like it would be a
  reasonable choice for the SHA1 length to use here.
 
 Reusing a configuration that is used to control something similar
 sounds sensible to me.
 
  The only possible
  problem is that it defaults to 40 and so might give an overly long
  result for some users.  Maybe you could use $autosellen but limit it
  to at most 12 or 16 or something like that.
 
 How is the thing that is automatically selected when going to a new
 commit used by the end user?  What is the reason why people may
 want to configure it?  I understand that this is the string that
 goes into the selection buffer, so presumably people are using this
 selection to paste elsewhere?  If so, that sounds like very similar
 to Beat's use case---perhaps if 40 is too long for Beat's use case
 as a sensible default, then it is also too long for its original use
 case?

It's used for pasting into commit messages and emails, and it's used
for pasting onto the command line when typing git commands.  For the
second, the length doesn't matter; the limit was added for the first
case.

 Or do you expect it to be common to want to use autosellen set to 40
 and Beat's abbrev len set to much shorter, e.g. 16?  If so they may
 deserve two different settings, with different defaults.

I would think that if $autosellen is 40 it's almost certainly because
the user hasn't found that control in the preferences window. :)

 Artificially limiting it to 12 or 16 does not sound all that
 sensible, though.

Adding --abbrev=$autosellen if $autosellen is not 40 sounds like it
would do what we want.

Paul.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-17 Thread Junio C Hamano
Paul Mackerras pau...@samba.org writes:

 We have an item in the preferences menu to control the SHA1 length
 that is automatically selected when going to a new commit.  It's
 stored in the variable $autosellen.  That seems like it would be a
 reasonable choice for the SHA1 length to use here.

Reusing a configuration that is used to control something similar
sounds sensible to me.

 The only possible
 problem is that it defaults to 40 and so might give an overly long
 result for some users.  Maybe you could use $autosellen but limit it
 to at most 12 or 16 or something like that.

How is the thing that is automatically selected when going to a new
commit used by the end user?  What is the reason why people may
want to configure it?  I understand that this is the string that
goes into the selection buffer, so presumably people are using this
selection to paste elsewhere?  If so, that sounds like very similar
to Beat's use case---perhaps if 40 is too long for Beat's use case
as a sensible default, then it is also too long for its original use
case?

Or do you expect it to be common to want to use autosellen set to 40
and Beat's abbrev len set to much shorter, e.g. 16?  If so they may
deserve two different settings, with different defaults.

Artificially limiting it to 12 or 16 does not sound all that
sensible, though.

Thanks.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-17 Thread Stefan Haller
Junio C Hamano gits...@pobox.com wrote:

 Beat Bolli dev+...@drbeat.li writes:
 
  When referring to earlier commits in commit messages or other text, one
  of the established formats is
 
  abbrev-sha (summary, author-date)
  ...
  +proc copysummary {} {
  +global rowmenuid commitinfo
  +
  +set id [string range $rowmenuid 0 7]
  +set info $commitinfo($rowmenuid)
  +set commit [lindex $info 0]
 
 7 hexdigits is not always an appropriate value for all projects.
 The minimum necessary to guarantee uniqueness varies on project, and
 it is not a good idea to hardcode such a small value.  Not-so-old
 Linux kernel history seems to use at least 12, for example.
 
 I believe that the one of the established formats comes from a
 git one alias I published somewhere long time ago, that did
 something like this:
 
   git show -s --abbrev=8 --pretty='format:%h (%s, %ai' $@ |
   sed -e 's/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9][0-9][0-9]$/)/'
 
 where the combination of --abbrev=8 and format:%h asks for a unique
 abbreviation that is at least 8 hexdigits long but can use more than
 8 if it is not long enough to uniquely identify the given commit.

For the intended use case of this feature (referring to earlier commits
in commit messages), guaranteeing uniqueness isn't sufficiant either.
What is unique at the time of creating the commit might no longer be
unique a few years later.

So one strategy would be to add one or two digits to what %h returns, to
give some future leeway; or rely on the user to configure core.abbrev
appropriatly for their project; or just make the hard-coded value
configurable, as Hannes suggests.

FWIW, a discussion of this that I find useful can be found here:
http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/.


-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-17 Thread Beat Bolli

On 2015-07-17 10:50, li...@haller-berlin.de wrote:

Junio C Hamano gits...@pobox.com wrote:


Beat Bolli dev+...@drbeat.li writes:

 When referring to earlier commits in commit messages or other 
text, one

 of the established formats is

 abbrev-sha (summary, author-date)
 ...
 +proc copysummary {} {
 +global rowmenuid commitinfo
 +
 +set id [string range $rowmenuid 0 7]
 +set info $commitinfo($rowmenuid)
 +set commit [lindex $info 0]

7 hexdigits is not always an appropriate value for all projects.
The minimum necessary to guarantee uniqueness varies on project, and
it is not a good idea to hardcode such a small value.  Not-so-old
Linux kernel history seems to use at least 12, for example.

I believe that the one of the established formats comes from a
git one alias I published somewhere long time ago, that did
something like this:

  git show -s --abbrev=8 --pretty='format:%h (%s, %ai' $@ |
  sed -e 's/ [012][0-9]:[0-5][0-9]:[0-5][0-9] 
[-+][0-9][0-9][0-9][0-9]$/)/'


where the combination of --abbrev=8 and format:%h asks for a unique
abbreviation that is at least 8 hexdigits long but can use more than
8 if it is not long enough to uniquely identify the given commit.


For the intended use case of this feature (referring to earlier 
commits

in commit messages), guaranteeing uniqueness isn't sufficiant either.
What is unique at the time of creating the commit might no longer be
unique a few years later.


This is true, but the purpose of the format with the summary text and 
date
is exactly to make it redundant enough that the hash doesn't have to be 
unique

in eternity.

So one strategy would be to add one or two digits to what %h returns, 
to

give some future leeway; or rely on the user to configure core.abbrev
appropriatly for their project; or just make the hard-coded value
configurable, as Hannes suggests.

FWIW, a discussion of this that I find useful can be found here:
http://blog.cuviper.com/2013/11/10/how-short-can-git-abbreviate/.

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-17 Thread Paul Mackerras
On Thu, Jul 16, 2015 at 05:29:25PM +0200, Beat Bolli wrote:
 When referring to earlier commits in commit messages or other text, one
 of the established formats is
 
 abbrev-sha (summary, author-date)
 
 Add a Copy commit summary command to the context menu that puts this
 text for the currently selected commit on the clipboard. This makes it
 easy for our users to create well-formatted commit references.

I really like this idea, but as others have noted, 8 characters may
not be the right choice for the SHA1 length in all cases.

We have an item in the preferences menu to control the SHA1 length
that is automatically selected when going to a new commit.  It's
stored in the variable $autosellen.  That seems like it would be a
reasonable choice for the SHA1 length to use here.  The only possible
problem is that it defaults to 40 and so might give an overly long
result for some users.  Maybe you could use $autosellen but limit it
to at most 12 or 16 or something like that.

Paul.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] gitk: Add a Copy commit summary command

2015-07-16 Thread Beat Bolli
When referring to earlier commits in commit messages or other text, one
of the established formats is

abbrev-sha (summary, author-date)

Add a Copy commit summary command to the context menu that puts this
text for the currently selected commit on the clipboard. This makes it
easy for our users to create well-formatted commit references.

Signed-off-by: Beat Bolli dev+...@drbeat.li
Cc: Paul Mackerras pau...@samba.org
---
 gitk-git/gitk | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9a2daf3..72a2756 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2617,6 +2617,7 @@ proc makewindow {} {
{mc Diff selected - this command {diffvssel 1}}
{mc Make patch command mkpatch}
{mc Create tag command mktag}
+   {mc Copy commit summary command copysummary}
{mc Write commit to file command writecommit}
{mc Create new branch command mkbranch}
{mc Cherry-pick this commit command cherrypick}
@@ -9341,6 +9342,19 @@ proc mktaggo {} {
 mktagcan
 }
 
+proc copysummary {} {
+global rowmenuid commitinfo
+
+set id [string range $rowmenuid 0 7]
+set info $commitinfo($rowmenuid)
+set commit [lindex $info 0]
+set date [formatdate [lindex $info 2]]
+set summary $id (\$commit\, $date)
+
+clipboard clear
+clipboard append $summary
+}
+
 proc writecommit {} {
 global rowmenuid wrcomtop commitinfo wrcomcmd NS
 
-- 
2.1.4
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-16 Thread Junio C Hamano
Beat Bolli dev+...@drbeat.li writes:

 When referring to earlier commits in commit messages or other text, one
 of the established formats is

 abbrev-sha (summary, author-date)
 ...
 +proc copysummary {} {
 +global rowmenuid commitinfo
 +
 +set id [string range $rowmenuid 0 7]
 +set info $commitinfo($rowmenuid)
 +set commit [lindex $info 0]

7 hexdigits is not always an appropriate value for all projects.
The minimum necessary to guarantee uniqueness varies on project, and
it is not a good idea to hardcode such a small value.  Not-so-old
Linux kernel history seems to use at least 12, for example.

I believe that the one of the established formats comes from a
git one alias I published somewhere long time ago, that did
something like this:

  git show -s --abbrev=8 --pretty='format:%h (%s, %ai' $@ |
  sed -e 's/ [012][0-9]:[0-5][0-9]:[0-5][0-9] [-+][0-9][0-9][0-9][0-9]$/)/'

where the combination of --abbrev=8 and format:%h asks for a unique
abbreviation that is at least 8 hexdigits long but can use more than
8 if it is not long enough to uniquely identify the given commit.

I do not offhand know how $commitinfo is populated, but perhaps you
can tweak that code to ask for both %H (for the full commit object
ID) and %h (for the unique abbreviation of appropriate length) and
store the value for %h to a new field in the $commitinfo($rowmenuid)
array, so that you do not have to have such a hard-coded truncation
here?

 +set date [formatdate [lindex $info 2]]
 +set summary $id (\$commit\, $date)
 +
 +clipboard clear
 +clipboard append $summary
 +}
 +
  proc writecommit {} {
  global rowmenuid wrcomtop commitinfo wrcomcmd NS
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2] gitk: Add a Copy commit summary command

2015-07-16 Thread Johannes Sixt

Am 16.07.2015 um 17:29 schrieb Beat Bolli:

When referring to earlier commits in commit messages or other text, one
of the established formats is

 abbrev-sha (summary, author-date)

Add a Copy commit summary command to the context menu that puts this
text for the currently selected commit on the clipboard. This makes it
easy for our users to create well-formatted commit references.

Signed-off-by: Beat Bolli dev+...@drbeat.li
Cc: Paul Mackerras pau...@samba.org
---
  gitk-git/gitk | 14 ++
  1 file changed, 14 insertions(+)

diff --git a/gitk-git/gitk b/gitk-git/gitk
index 9a2daf3..72a2756 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2617,6 +2617,7 @@ proc makewindow {} {
{mc Diff selected - this command {diffvssel 1}}
{mc Make patch command mkpatch}
{mc Create tag command mktag}
+   {mc Copy commit summary command copysummary}
{mc Write commit to file command writecommit}
{mc Create new branch command mkbranch}
{mc Cherry-pick this commit command cherrypick}
@@ -9341,6 +9342,19 @@ proc mktaggo {} {
  mktagcan
  }

+proc copysummary {} {
+global rowmenuid commitinfo
+
+set id [string range $rowmenuid 0 7]


You abbreviate the commit name to 7 characters. This is too short for 
certain repositories to remain unique. In my group, it is customary to 
abbreviate to 8 charaters. This reduces the usefulness for my use. If 
you don't want to make this a configuration I would suggest to aim for a 
longer commit name as it is simpler to delete excess letters after 
pasting than to add back the missing ones.


Except for this, I like the idea.


+set info $commitinfo($rowmenuid)
+set commit [lindex $info 0]
+set date [formatdate [lindex $info 2]]
+set summary $id (\$commit\, $date)
+
+clipboard clear
+clipboard append $summary
+}
+
  proc writecommit {} {
  global rowmenuid wrcomtop commitinfo wrcomcmd NS




-- Hannes

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html