All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Provide for config to specify tags not to abbreviate
@ 2016-11-08  0:52 Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 1/6] gitk: Internal: drawtags: Abolish "singletag" variable Ian Jackson
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Junio C Hamano, Paul Mackerras

Hi.

Please find in the following mails patches which provide a way to make
gitk display certain tags in full, even if they would normally be
abbreviated.

There are four patches to gitk, three to prepare the ground, and one
to introduce the new feature.

There is one patch for git, to just document the new config variable.

I hope this is the right way to submit this series.  Thanks for your
attention.


As I say in the patch "gitk: Provide for config to specify tags not to
abbreviate":

The config setting is in git config logs.* rather than gitk's
own configuration, because:

 - Tools which manage git trees may want to set this, depending
   on their knowledge of the nature of the tags likely to be
   present;

 - Whether this property ought to be set is mostly a property of the
   contents of the tag namespaces in the tree, not a user preference.
   (Although of course user preferences are supported.)

 - Other git utilities (or out of tree utilities) may want to
   reference this setting for their own display purposes.

There will be another, separate, patch to the `git' tree to document
this config option.

Background motivation:

Debian's dgit archive gateway tool generates and uses tags called
archive/debian/VERSION.  If such a tag refers to a Debian source tree,
it is probably very interesting because it refers to a version
actually uploaded to Debian by the Debian package maintainer.

We would therefore like a way to specify that such tags should be
displayed in full.  dgit will be able to set an appropriate config
setting in the trees it deals with.



Ian Jackson (4):
  gitk: Internal: drawtags: Abolish "singletag" variable
  gitk: Internal: drawtags: Idempotently reset "ntags"
  gitk: drawtags: Introduce concept of unabbreviated marks
  gitk: Provide for config to specify tags not to abbreviate

 gitk | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)


Ian Jackson (1):
  config docs: Provide for config to specify tags not to abbreviate

 Documentation/config.txt | 8 ++++++++
 1 file changed, 8 insertions(+)


-- 
2.10.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH GITK 1/6] gitk: Internal: drawtags: Abolish "singletag" variable
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
@ 2016-11-08  0:52 ` Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 2/6] gitk: Internal: drawtags: Idempotently reset "ntags" Ian Jackson
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Paul Mackerras

We are going to want to make the contents of `marks' somewhat more
complicated in a moment, so it won't be possible to use what is
effectively a single variable to represent the status of the whole of
the non-heads part of the marks list.

Luckily the strings that replace actual tag names, in the `singletag'
case, are not themselves valid tag names.  So they can be detected
directly.

(Also, `singletag' was not quite right anyway: really it meant that
the tag name(s) had been abbreviated.)

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 gitk | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/gitk b/gitk
index 805a1c7..42fa41a 100755
--- a/gitk
+++ b/gitk
@@ -6570,7 +6570,6 @@ proc drawtags {id x xt y1} {
 	if {$ntags > $maxtags ||
 	    [totalwidth $marks mainfont $extra] > $maxwidth} {
 	    # show just a single "n tags..." tag
-	    set singletag 1
 	    if {$ntags == 1} {
 		set marks [list "tag..."]
 	    } else {
@@ -6620,7 +6619,7 @@ proc drawtags {id x xt y1} {
 		       $xr $yt $xr $yb $xl $yb $x [expr {$yb - $delta}] \
 		       -width 1 -outline $tagoutlinecolor -fill $tagbgcolor \
 		       -tags tag.$id]
-	    if {$singletag} {
+	    if {[regexp {^tag\.\.\.|^\d+ } $tag]} {
 		set tagclick [list showtags $id 1]
 	    } else {
 		set tagclick [list showtag $tag_quoted 1]
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH GITK 2/6] gitk: Internal: drawtags: Idempotently reset "ntags"
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 1/6] gitk: Internal: drawtags: Abolish "singletag" variable Ian Jackson
@ 2016-11-08  0:52 ` Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 3/6] gitk: drawtags: Introduce concept of unabbreviated marks Ian Jackson
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Paul Mackerras

The previous code tracked its change to the length of `marks' by
updateing the variable `ntags'.  This is a bit fragile and cumbersome,
and we are going to want to modify `marks' some more in a moment.

Instead, simply reset ntags to the length of marks, after we have
possibly done any needed abbreviation.

No functional change.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 gitk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 42fa41a..31aecda 100755
--- a/gitk
+++ b/gitk
@@ -6575,9 +6575,10 @@ proc drawtags {id x xt y1} {
 	    } else {
 		set marks [list [format "%d tags..." $ntags]]
 	    }
-	    set ntags 1
 	}
     }
+    set ntags [llength $marks]
+
     if {[info exists idheads($id)]} {
 	set marks [concat $marks $idheads($id)]
 	set nheads [llength $idheads($id)]
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH GITK 3/6] gitk: drawtags: Introduce concept of unabbreviated marks
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 1/6] gitk: Internal: drawtags: Abolish "singletag" variable Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 2/6] gitk: Internal: drawtags: Idempotently reset "ntags" Ian Jackson
@ 2016-11-08  0:52 ` Ian Jackson
  2016-11-08  0:52 ` [PATCH GITK 4/6] gitk: Provide for config to specify tags not to abbreviate Ian Jackson
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Paul Mackerras

We are going to want to show some tags in full, even if they are long
or there are other tags.  Do this by filtering the tags into
`marks_unabbrev' and `marks'.  `marks_unabbrev' bypasses the tag
abbreviation, and is put on the front of the marks array after any
abbreviation has been done.

No functional change right now because no tags are considered
`unabbrev'.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 gitk | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/gitk b/gitk
index 31aecda..d76f1e3 100755
--- a/gitk
+++ b/gitk
@@ -6546,6 +6546,10 @@ proc totalwidth {l font extra} {
     return $tot
 }
 
+proc tag_want_unabbrev {tag} {
+    return 0
+}
+
 proc drawtags {id x xt y1} {
     global idtags idheads idotherrefs mainhead
     global linespc lthickness
@@ -6564,8 +6568,16 @@ proc drawtags {id x xt y1} {
     set delta [expr {int(0.5 * ($linespc - $lthickness))}]
     set extra [expr {$delta + $lthickness + $linespc}]
 
+    set marks_unabbrev {}
     if {[info exists idtags($id)]} {
-	set marks $idtags($id)
+	set marks {}
+	foreach tag $idtags($id) {
+	    if {[tag_want_unabbrev $tag]} {
+		lappend marks_unabbrev $tag
+	    } else {
+		lappend marks $tag
+	    }
+	}
 	set ntags [llength $marks]
 	if {$ntags > $maxtags ||
 	    [totalwidth $marks mainfont $extra] > $maxwidth} {
@@ -6577,6 +6589,7 @@ proc drawtags {id x xt y1} {
 	    }
 	}
     }
+    set marks [concat $marks_unabbrev $marks]
     set ntags [llength $marks]
 
     if {[info exists idheads($id)]} {
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH GITK 4/6] gitk: Provide for config to specify tags not to abbreviate
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
                   ` (2 preceding siblings ...)
  2016-11-08  0:52 ` [PATCH GITK 3/6] gitk: drawtags: Introduce concept of unabbreviated marks Ian Jackson
@ 2016-11-08  0:52 ` Ian Jackson
  2016-11-08  0:52 ` [PATCH 5/6] config docs: " Ian Jackson
  2016-11-08  0:54 ` [PATCH 0/6] " Ian Jackson
  5 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Paul Mackerras

Tags matching a new multi-valued config option log.noAbbrevTags
are not abbreviated.

The config setting is in git config logs.* rather than gitk's
own configuration, because:

 - Tools which manage git trees may want to set this, depending
   on their knowledge of the nature of the tags likely to be
   present;

 - Whether this property ought to be set is mostly a property of the
   contents of the tag namespaces in the tree, not a user preference.
   (Although of course user preferences are supported.)

 - Other git utilities (or out of tree utilities) may want to
   reference this setting for their own display purposes.

There will be another, separate, patch to the `git' tree to document
this config option.

Background motivation:

Debian's dgit archive gateway tool generates and uses tags called
archive/debian/VERSION.  If such a tag refers to a Debian source tree,
it is probably very interesting because it refers to a version
actually uploaded to Debian by the Debian package maintainer.

We would therefore like a way to specify that such tags should be
displayed in full.  dgit will be able to set an appropriate config
setting in the trees it deals with.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 gitk | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/gitk b/gitk
index d76f1e3..515d7b0 100755
--- a/gitk
+++ b/gitk
@@ -6547,6 +6547,14 @@ proc totalwidth {l font extra} {
 }
 
 proc tag_want_unabbrev {tag} {
+    global noabbrevtags
+    # noabbrevtags was reversed when we read config, so take first match
+    foreach pat $noabbrevtags {
+	set inverted [regsub {^\^} $pat {} pat]
+	if {[string match $pat $tag]} {
+	    return [expr {!$inverted}]
+	}
+    }
     return 0
 }
 
@@ -12138,6 +12146,11 @@ set tclencoding [tcl_encoding $gitencoding]
 if {$tclencoding == {}} {
     puts stderr "Warning: encoding $gitencoding is not supported by Tcl/Tk"
 }
+set noabbrevtags {}
+catch {
+    set noabbrevtags [exec git config --get-all log.noAbbrevTags]
+}
+set noabbrevtags [lreverse [split $noabbrevtags "\n"]]
 
 set gui_encoding [encoding system]
 catch {
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
                   ` (3 preceding siblings ...)
  2016-11-08  0:52 ` [PATCH GITK 4/6] gitk: Provide for config to specify tags not to abbreviate Ian Jackson
@ 2016-11-08  0:52 ` Ian Jackson
  2016-11-08  2:28   ` Jacob Keller
  2016-11-08  0:54 ` [PATCH 0/6] " Ian Jackson
  5 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:52 UTC (permalink / raw)
  To: git; +Cc: Ian Jackson, Junio C Hamano

Tags matching a new multi-valued config option log.noAbbrevTags
should not be abbreviated.  Currently this config option is
used only by gitk (and the patch to gitk will come via the
gitk maintainer tree).

The config setting is in git config logs.* rather than gitk's
own configuration, because:

 - Tools which manage git trees may want to set this, depending
   on their knowledge of the nature of the tags likely to be
   present;

 - Whether this property ought to be set is mostly a property of the
   contents of the tag namespaces in the tree, not a user preference.
   (Although of course user preferences are supported.)

 - Other git utilities (or out of tree utilities) may want to
   reference this setting for their own display purposes.

Background motivation:

Debian's dgit archive gateway tool generates and uses tags called
archive/debian/VERSION.  If such a tag refers to a Debian source tree,
it is probably very interesting because it refers to a version
actually uploaded to Debian by the Debian package maintainer.

We would therefore like a way to specify that such tags should be
displayed in full.  dgit will be able to set an appropriate config
setting in the trees it deals with.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
---
 Documentation/config.txt | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index a0ab66a..6aade4f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2002,6 +2002,14 @@ log.abbrevCommit::
 	linkgit:git-whatchanged[1] assume `--abbrev-commit`. You may
 	override this option with `--no-abbrev-commit`.
 
+log.noAbbrevTags::
+	Each value is a glob pattern, specifying tag nammes which
+	should always be displayed in full, even when other tags may
+	be omitted or abbreviated (for example, by linkgit:gitk[1]).
+	Values starting with `^` specify tags which should be
+	abbreviated.  The order is important: the last match, in the
+	most-local configuration, wins.
+
 log.date::
 	Set the default date-time mode for the 'log' command.
 	Setting a value for log.date is similar to using 'git log''s
-- 
2.10.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] Provide for config to specify tags not to abbreviate
  2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
                   ` (4 preceding siblings ...)
  2016-11-08  0:52 ` [PATCH 5/6] config docs: " Ian Jackson
@ 2016-11-08  0:54 ` Ian Jackson
  2016-11-08  9:19   ` Markus Hitter
  5 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-08  0:54 UTC (permalink / raw)
  To: Ian Jackson; +Cc: git, Junio C Hamano, Paul Mackerras

Ian Jackson writes ("[PATCH 0/6] Provide for config to specify tags not to abbreviate"):
> Please find in the following mails patches which provide a way to make
> gitk display certain tags in full, even if they would normally be
> abbreviated.
> 
> There are four patches to gitk, three to prepare the ground, and one
> to introduce the new feature.
>
> There is one patch for git, to just document the new config variable.

The eagle-eyed reader will spot that that makes 5 patches, not 6.
There are indeed only 5.  The subject mentioning 6 is a mistake -
sorry.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-08  0:52 ` [PATCH 5/6] config docs: " Ian Jackson
@ 2016-11-08  2:28   ` Jacob Keller
  2016-11-08 10:51     ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Jacob Keller @ 2016-11-08  2:28 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Git mailing list, Junio C Hamano

On Mon, Nov 7, 2016 at 4:52 PM, Ian Jackson
<ijackson@chiark.greenend.org.uk> wrote:
> +log.noAbbrevTags::
> +       Each value is a glob pattern, specifying tag nammes which
> +       should always be displayed in full, even when other tags may
> +       be omitted or abbreviated (for example, by linkgit:gitk[1]).
> +       Values starting with `^` specify tags which should be
> +       abbreviated.  The order is important: the last match, in the
> +       most-local configuration, wins.
> +

It seems weird that this description implies some sort of behavior
change in core git itself, but in fact is only used as a reference for
other tools that may or may not honor it. I guess the reasoning here
is to try to get other external tools that abbreviate tags to also
honor this? But it still seems pretty weird to have a documented
config that has no code in core git to honor it...

Thanks,
Jake

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] Provide for config to specify tags not to abbreviate
  2016-11-08  0:54 ` [PATCH 0/6] " Ian Jackson
@ 2016-11-08  9:19   ` Markus Hitter
  2016-11-08 13:42     ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Markus Hitter @ 2016-11-08  9:19 UTC (permalink / raw)
  To: Ian Jackson; +Cc: git, Junio C Hamano, Paul Mackerras

Am 08.11.2016 um 01:54 schrieb Ian Jackson:
> Please find in the following mails patches which provide a way to make
> gitk display certain tags in full, even if they would normally be
> abbreviated.

TBH, I see a violation of tool independence with the choice of preference storage. Abbreviation of tags isn't a property of the repository, but a pure visual thing (screen real estate, whatever), so it should be handled by the tool doing the visuals, only.

Your use case looks like a nice opportunity for

- adding a Gitk user preference on how long displayed tags are allowed to be (instead of distinguishing between abbreviated and unabbreviated ones; set it to 999 for your use case) and/or

- even better, abbreviate them depending on the size of the visible area, like a web browser would do, and/or

- considering whether tags should be abbreviated on the left instead of on the right and/or

- finding a mechanism to show them in full length even on small visible areas.

The latter could be done by a tooltip appearing when hovering with the mouse over an abbreviated tag or by allowing multiple lines for a single commit in the list of commits.

Trying to enforce long names just means they're not cut off by the abbreviation algorithm, but by the right boundary of the visible area.


My $0.02,
Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-08  2:28   ` Jacob Keller
@ 2016-11-08 10:51     ` Ian Jackson
  2016-11-08 21:57       ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-08 10:51 UTC (permalink / raw)
  To: Jacob Keller; +Cc: Git mailing list, Junio C Hamano

Jacob Keller writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> On Mon, Nov 7, 2016 at 4:52 PM, Ian Jackson
> <ijackson@chiark.greenend.org.uk> wrote:
> > +log.noAbbrevTags::
> > +       Each value is a glob pattern, specifying tag nammes which
> > +       should always be displayed in full, even when other tags may
> > +       be omitted or abbreviated (for example, by linkgit:gitk[1]).
> > +       Values starting with `^` specify tags which should be
> > +       abbreviated.  The order is important: the last match, in the
> > +       most-local configuration, wins.
> > +
> 
> It seems weird that this description implies some sort of behavior
> change in core git itself, but in fact is only used as a reference for
> other tools that may or may not honor it. I guess the reasoning here
> is to try to get other external tools that abbreviate tags to also
> honor this? But it still seems pretty weird to have a documented
> config that has no code in core git to honor it...

Thanks for your attention.

Yes, I agree that it does seem weird.  But the alternatives seem
worse.  I think it's probably best if options like this (currently
only honoured by out-of-core tools but of general usefulness) are
collected together here.

There is a precedent: `git config gui.encoding' is, according to the
documentation, honoured only by git-gui and gitk.

Calling the config option `gitk.noAbbrevTags' would be possible but
that would invite everyone else to invent their own, which would be
quite annoying.  (Also, gitk does not have any gitk-specific git
config options right now, AIUI.  It does honour `git config
gui.encoding'.)

Would it help to add a sentence to the documentation saying that this
is currently only honoured by gitk ?  (The paragraph for gui.encoding
says something similar.)  Of course I don't know who else abbreviates
tags, but as they gain support they could be added to the docs.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 0/6] Provide for config to specify tags not to abbreviate
  2016-11-08  9:19   ` Markus Hitter
@ 2016-11-08 13:42     ` Ian Jackson
  0 siblings, 0 replies; 18+ messages in thread
From: Ian Jackson @ 2016-11-08 13:42 UTC (permalink / raw)
  To: Markus Hitter; +Cc: git, Junio C Hamano, Paul Mackerras

Markus Hitter writes ("Re: [PATCH 0/6] Provide for config to specify tags not to abbreviate"):
> TBH, I see a violation of tool independence with the choice of
> preference storage. Abbreviation of tags isn't a property of the
> repository, but a pure visual thing (screen real estate, whatever),
> so it should be handled by the tool doing the visuals, only.

As I explained in my cover letter, the set of tags which are important
enough not to abbreviate, even if they would normally be abbreviated,
is indeed a property of the repository.

The alternative would be for a tool like gitk to grow an
ever-increasing set of heuristics.  Or, worse, for a tool like dgit
(which knows that archive/* are special) to edit the user's personal
gitk settings.

> Your use case looks like a nice opportunity for
> 
> - adding a Gitk user preference on how long displayed tags are
>   allowed to be (instead of distinguishing between abbreviated and
>   unabbreviated ones; set it to 999 for your use case) and/or

This would be wrong, because it's only certain tags that ought not to
be abbreviated.  The right way to identify those tags is by 1. what
repo they are in 2. what their name is.  (It might be possible to
identify them by content or something - for example, the interesting
archive/* tags all refer to commits whose trees contain debian/ - but
that is getting quite out of hand.)

What you propose are possible general improvements to the abbreviation
system in gitk.  But they do not address the fundamental point that
some tags are much more interesting than others.  It is this latter
point that I am trying to deal with.

Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-08 10:51     ` Ian Jackson
@ 2016-11-08 21:57       ` Jeff King
  2016-11-09  1:41         ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2016-11-08 21:57 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Jacob Keller, Git mailing list, Junio C Hamano

On Tue, Nov 08, 2016 at 10:51:33AM +0000, Ian Jackson wrote:

> Yes, I agree that it does seem weird.  But the alternatives seem
> worse.  I think it's probably best if options like this (currently
> only honoured by out-of-core tools but of general usefulness) are
> collected together here.
> 
> There is a precedent: `git config gui.encoding' is, according to the
> documentation, honoured only by git-gui and gitk.

Yeah, I think git's config system was always designed to carry options
for porcelains outside of git-core itself. So your new option fits into
that.

I think the two things I found weird were:

  - it's in the "log" section, which makes me think it's an option for
    git-log. But it's not. I'm not sure what the _right_ section is, but
    hopefully it would make it clear that this is command-agnostic.

    Something like "gui.abbrevTags" might be OK (and as you note, has
    precedence). But of course it's possible that a command like "tig"
    could learn to support it.  I'm not sure if that counts as a GUI or
    not. :)

  - The description talks about tag abbreviation, but doesn't really
    define it. Not being a gitk user, it was hard for me to figure out
    whether this was even relevant. Does it mean turning
    "refs/tags/v1.0" into "1.0"? From the rest of the series, it sounds
    like no. That should be more clear from the documentation.

-Peff

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-08 21:57       ` Jeff King
@ 2016-11-09  1:41         ` Ian Jackson
  2016-11-09  5:55           ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-09  1:41 UTC (permalink / raw)
  To: Jeff King; +Cc: Jacob Keller, Git mailing list, Junio C Hamano, Paul Mackerras

Jeff King writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> Yeah, I think git's config system was always designed to carry options
> for porcelains outside of git-core itself. So your new option fits into
> that.

Good, thanks.

> I think the two things I found weird were:
> 
>   - it's in the "log" section, which makes me think it's an option for
>     git-log. But it's not. I'm not sure what the _right_ section is, but
>     hopefully it would make it clear that this is command-agnostic.
> 
>     Something like "gui.abbrevTags" might be OK (and as you note, has
>     precedence). But of course it's possible that a command like "tig"
>     could learn to support it.  I'm not sure if that counts as a GUI or
>     not. :)

I don't really have an opinion about the name.  gui.abbrevTags would
be a possibility.  (It's a bit odd that implicitly, the default would
be `*'.)

>   - The description talks about tag abbreviation, but doesn't really
>     define it. Not being a gitk user, it was hard for me to figure out
>     whether this was even relevant. Does it mean turning
>     "refs/tags/v1.0" into "1.0"? From the rest of the series, it sounds
>     like no. That should be more clear from the documentation.

I can do that, sure.

By default, gitk doesn't like to use much screen real estate for tags.
If there are long tag names, or many tags, it shows them all as a
single small indication saying just `<tag...|' or whatever with the
literal `tag...', not with the tag value.

Maybe a better name would be
   gui.alwaysShowTags
?

I'm happy to be just told what the name ought to be, if the gitk and
git maintainers can agree.  It seems largely a matter of taste.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-09  1:41         ` Ian Jackson
@ 2016-11-09  5:55           ` Junio C Hamano
  2016-11-09 10:51             ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-11-09  5:55 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

>> I think the two things I found weird were:
>> 
>>   - it's in the "log" section, which makes me think it's an option for
>>     git-log. But it's not. I'm not sure what the _right_ section is, but
>>     hopefully it would make it clear that this is command-agnostic.
>> 
>>     Something like "gui.abbrevTags" might be OK (and as you note, has
>>     precedence). But of course it's possible that a command like "tig"
>>     could learn to support it.  I'm not sure if that counts as a GUI or
>>     not. :)
>
> I don't really have an opinion about the name.  gui.abbrevTags would
> be a possibility.  (It's a bit odd that implicitly, the default would
> be `*'.)

I have trouble with both "log" and "abbrev" in the name.  Perhaps I
am biased by our recent discussion on a feature in the core that we
use the word "abbrev" to describe, but I fear that most Git users,
when told the word, would imagine the act of shortening 40-hex full
object name down to shorter but still unique prefix, not the "this
refname is too long, so let's show only the first few letters in GUI
label".

And I do not think we would want "log" or any core side Porcelain
command to have too many "information losing" options like this
"truncate refnames down to a point where it is no longer unique and
meaningful".  GUI tools can get away with doing sos because they can
arrange these truncated labels to react to end-user input (e.g. the
truncated Tag in the history display of gitk could be made to react
to mouse-over and pop-up to show a full name, for example), but the
output from the core side is pretty much fixed once it is emitted.

So my first preference would be to teach gitk such a "please
clarify" UI-reaction, if it does not know how to do so yet.  There
is no need for a configuration variable anywhere with this approach.

If you do want to add a configuration to show fuller name in the
tag, which would make it unnecessary for the user to do "please
clarify, as I am hovering over what I want to get details of"
action, that may also be a good way to go.  But I think the right
place to do so would be Edit -> Preferences menu in Gitk, and the
settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
gitk-specific place.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-09  5:55           ` Junio C Hamano
@ 2016-11-09 10:51             ` Ian Jackson
  2016-11-09 23:13               ` Junio C Hamano
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-09 10:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras

Junio C Hamano writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> And I do not think we would want "log" or any core side Porcelain
> command to have too many "information losing" options like this
> "truncate refnames down to a point where it is no longer unique and
> meaningful".  GUI tools can get away with doing sos because they can
> arrange these truncated labels to react to end-user input (e.g. the
> truncated Tag in the history display of gitk could be made to react
> to mouse-over and pop-up to show a full name, for example), but the
> output from the core side is pretty much fixed once it is emitted.
> 
> So my first preference would be to teach gitk such a "please
> clarify" UI-reaction, if it does not know how to do so yet.  There
> is no need for a configuration variable anywhere with this approach.

gitk already has a way for the user to find out what the elided tag
names are.  The underlying difficulty is that the situation that the
gitk behaviour is designed for (long tag names, perhaps several to a
commit, not particularly interesting), is not applicable to these
particular tags.

Whether the tag is `particularly interesting' depends, as I say, on
both what tree it is in, and on its name.  It might be appropriate for
terminal-based tools to highlight these tags too, or show them when
tags are not normally displayed.

`core.interestingTags' ?

> If you do want to add a configuration to show fuller name in the
> tag, which would make it unnecessary for the user to do "please
> clarify, as I am hovering over what I want to get details of"
> action, that may also be a good way to go.

I think in my use case, which I hope to become common within Debian,
this is going to be essential.

>  But I think the right
> place to do so would be Edit -> Preferences menu in Gitk, and the
> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
> gitk-specific place.

This is not correct, because as I have explained, this should be a
per-tree configuration:

If it can't be a `git config' option, even `git config gui.something',
then I guess I will have to teach gitk to read a config file in
GIT_DIR too.  But I think that is silly given that git already has a
config file reading system which handles per-tree configs.

If we can't get agreement from the git-core developers on a config to
be used, and documented, for any tool which has similar behaviour, I
think the right answer is `git config gitk.<something>', which would
be documented in gitk.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-09 10:51             ` Ian Jackson
@ 2016-11-09 23:13               ` Junio C Hamano
  2016-11-09 23:31                 ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2016-11-09 23:13 UTC (permalink / raw)
  To: Ian Jackson; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras

Ian Jackson <ijackson@chiark.greenend.org.uk> writes:

>> But I think the right
>> place to do so would be Edit -> Preferences menu in Gitk, and the
>> settings will be stored in ~/.gitk or ~/.config/git/gitk or whatever
>> gitk-specific place.
>
> This is not correct, because as I have explained, this should be a
> per-tree configuration:

I do not have fundamental opposition to make it part of .git/config,
but the name "gitk.something" or if you are enhancing git-gui at the
time perhaps "gui.something" would be appropriate.  

But it is still silly to have this kind of information that is very
specific to Gitk in two places, one that is pretty Gitk specific
that core-git does not know anything about, the other that are part
of the configuration storage of the core-git.  In the longer term,
it is necessary for them to be accessible from gitk's "Edit ->
Preferences" mechanism somehow, I would think, rather than forcing
users to sometimes go to GUI to tweak and sometimes run "git config".

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-09 23:13               ` Junio C Hamano
@ 2016-11-09 23:31                 ` Ian Jackson
  2016-11-10  0:36                   ` Markus Hitter
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2016-11-09 23:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras

Junio C Hamano writes ("Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate"):
> Ian Jackson <ijackson@chiark.greenend.org.uk> writes:
> > This is not correct, because as I have explained, this should be a
> > per-tree configuration:
> 
> I do not have fundamental opposition to make it part of .git/config,
> but the name "gitk.something" or if you are enhancing git-gui at the
> time perhaps "gui.something" would be appropriate.  
> 
> But it is still silly to have this kind of information that is very
> specific to Gitk in two places, one that is pretty Gitk specific
> that core-git does not know anything about, the other that are part
> of the configuration storage of the core-git.  In the longer term,
> it is necessary for them to be accessible from gitk's "Edit ->
> Preferences" mechanism somehow, I would think, rather than forcing
> users to sometimes go to GUI to tweak and sometimes run "git config".

I am proposing to set this configuration setting automatically in
dgit.  Other tools that work with particular git tags would do the
same.  There would be no need for users to do anything.

Having this as an option in a menu would be quite wrong, because it
would end up with the user and the tooling fighting.  This is why I
don't want to put this in gitk's existing config file mechanism.

It would be wrong for dgit to edit the user's gitk config file, for
many reasons.

To put it another way, this setting is a way for a tool like dgit to
communicate with gitk (or other programs which have to make guesses
about how prominently to present certain information to the user).
It's not intended to be a way for users, certainly not non-expert
users, to communicate with gitk.

The way I have structured my proposed patches in gitk would make it
easy to provide a gui option to adjust these settings.  Such a gui
option ought to save its value in the gitk config file, and those
values ought to override what comes from `git config'.

But such a system would not obviate the need for a legitimate way for
programs like dgit to communicate with gitk.

Thanks,
Ian.

-- 
Ian Jackson <ijackson@chiark.greenend.org.uk>   These opinions are my own.

If I emailed you from an address @fyvzl.net or @evade.org.uk, that is
a private address which bypasses my fierce spamfilter.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/6] config docs: Provide for config to specify tags not to abbreviate
  2016-11-09 23:31                 ` Ian Jackson
@ 2016-11-10  0:36                   ` Markus Hitter
  0 siblings, 0 replies; 18+ messages in thread
From: Markus Hitter @ 2016-11-10  0:36 UTC (permalink / raw)
  To: Ian Jackson, Junio C Hamano
  Cc: Jeff King, Jacob Keller, Git mailing list, Paul Mackerras

Am 10.11.2016 um 00:31 schrieb Ian Jackson:
> I am proposing to set this configuration setting automatically in
> dgit.  Other tools that work with particular git tags would do the
> same.  There would be no need for users to do anything.
> 
> Having this as an option in a menu would be quite wrong, because it
> would end up with the user and the tooling fighting.  This is why I
> don't want to put this in gitk's existing config file mechanism.

Having this conversation watched for a while I get the impression that your point is essentially about introducing another type of references, next to branches and (ordinary) tags. Let's call them "interesting tags", "itags".

The logical path to get this, IMHO, isn't to add some configuration variable, but to store such interesting tags in .git/refs/itags/, just like the other reference types. Then one would create such interesting tags with

  git tag -i <name>
or
  git tag --interesting <name>

To reduce the backwards compatibility problem these itags could be stored in .git/refs/tags as well, itag-aware tools would sort the duplicates out.


Markus

-- 
- - - - - - - - - - - - - - - - - - -
Dipl. Ing. (FH) Markus Hitter
http://www.jump-ing.de/

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2016-11-10  0:36 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-08  0:52 [PATCH 0/6] Provide for config to specify tags not to abbreviate Ian Jackson
2016-11-08  0:52 ` [PATCH GITK 1/6] gitk: Internal: drawtags: Abolish "singletag" variable Ian Jackson
2016-11-08  0:52 ` [PATCH GITK 2/6] gitk: Internal: drawtags: Idempotently reset "ntags" Ian Jackson
2016-11-08  0:52 ` [PATCH GITK 3/6] gitk: drawtags: Introduce concept of unabbreviated marks Ian Jackson
2016-11-08  0:52 ` [PATCH GITK 4/6] gitk: Provide for config to specify tags not to abbreviate Ian Jackson
2016-11-08  0:52 ` [PATCH 5/6] config docs: " Ian Jackson
2016-11-08  2:28   ` Jacob Keller
2016-11-08 10:51     ` Ian Jackson
2016-11-08 21:57       ` Jeff King
2016-11-09  1:41         ` Ian Jackson
2016-11-09  5:55           ` Junio C Hamano
2016-11-09 10:51             ` Ian Jackson
2016-11-09 23:13               ` Junio C Hamano
2016-11-09 23:31                 ` Ian Jackson
2016-11-10  0:36                   ` Markus Hitter
2016-11-08  0:54 ` [PATCH 0/6] " Ian Jackson
2016-11-08  9:19   ` Markus Hitter
2016-11-08 13:42     ` Ian Jackson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.