| Message ID | da9321b1bd56aafd16c8dcb99d5d628b79e2244e.1576100147.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gitk: use --pretty=reference for copysummary | expand |
On Wed, Dec 11, 2019 at 01:39:50PM -0800, Denton Liu wrote: > In an earlier commit[1], git learned the 'reference' pretty format. > Update copysummary to use this pretty format instead of manually > reimplementing it as a format string. > > With this change, we lose the double-quotes surrounding the commit > subject but it seems the consensus is that the unquoted form is used > more often anyway[2] so this change should be acceptable. > > Since gitk and git are usually packaged and distributed together, their > versions should be in sync so we should not have to worry a newer gitk > running on top of an older version of git that doesn't support the > 'reference' pretty format. In fact my policy is not to do this (introduce a change to gitk that means it requires the very latest git). I would want the code either to test the git version (which the code already does in other places) or handle failure gracefully and fall back to the old command. Paul.
Paul Mackerras <paulus@ozlabs.org> writes: > On Wed, Dec 11, 2019 at 01:39:50PM -0800, Denton Liu wrote: >> In an earlier commit[1], git learned the 'reference' pretty format. >> Update copysummary to use this pretty format instead of manually >> reimplementing it as a format string. >> >> With this change, we lose the double-quotes surrounding the commit >> subject but it seems the consensus is that the unquoted form is used >> more often anyway[2] so this change should be acceptable. >> >> Since gitk and git are usually packaged and distributed together, their >> versions should be in sync so we should not have to worry a newer gitk >> running on top of an older version of git that doesn't support the >> 'reference' pretty format. > > In fact my policy is not to do this (introduce a change to gitk that > means it requires the very latest git). I would want the code either > to test the git version (which the code already does in other places) > or handle failure gracefully and fall back to the old command. For a case like this one, the policy would mean that a single liner patch like this will never be accepted, right? After all, the code that would be used as a fallback for older Git is very simple so it is almost pointless to add a check for feature with conditional. We can just use the fallback code always, which is essentially to keep the current code. It is a tangent, but arguably the current code is easier to extend. I can even see somebody arguing for adding a gitk.summaryformat configuration variable, whose value would default to "%h (%s, %ad)" when missing---that can be quite straightforward to do without Denton's patch. So I dunno.
diff --git a/gitk b/gitk index abe4805ade..8bf198e338 100755 --- a/gitk +++ b/gitk @@ -9429,8 +9429,7 @@ proc mktaggo {} { proc copysummary {} { global rowmenuid autosellen - set format "%h (\"%s\", %ad)" - set cmd [list git show -s --pretty=format:$format --date=short] + set cmd [list git show -s --pretty=reference] if {$autosellen < 40} { lappend cmd --abbrev=$autosellen }
In an earlier commit[1], git learned the 'reference' pretty format. Update copysummary to use this pretty format instead of manually reimplementing it as a format string. With this change, we lose the double-quotes surrounding the commit subject but it seems the consensus is that the unquoted form is used more often anyway[2] so this change should be acceptable. Since gitk and git are usually packaged and distributed together, their versions should be in sync so we should not have to worry a newer gitk running on top of an older version of git that doesn't support the 'reference' pretty format. [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> --- Beat Bolli sent a series out earlier that did the exact same thing[3]. Since they haven't replied yet, I'll send out the version that I've been cooking for a while now since I think the commit message looks a bit better too and also it's based on top of Paul's tree. [3]: https://lore.kernel.org/git/20191209182534.309884-1-dev+git@drbeat.li/ gitk | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)