Subject: Re: [PATCH] gitk: add a checkbox to control the visibility of tags

From: Lukasz Stelmach <[email-masked]>
Date: Sun, 02 Dec 2012 22:25:52 +0100

W dniu 02.12.2012 03:16, Junio C Hamano pisze:
> Łukasz Stelmach <[email-masked]> writes:
> 
>> Enable hiding of tags displayed in the tree as yellow labels.
>> If a repository is used together with a system like Gerrit
>> there may be quite a lot of tags used to control building
>> and there may be hardly any place left for commit subjects.
>>
>> Signed-off-by: Łukasz Stelmach <[email-masked]>
>> ---
> 
> Paul, this patch is not done against your tree (does not have gitk
> at the top-level),

I did it on the master from github. Should I rebase it onto something else?

> but other than that, the change mimics the way
> existing hideremoes is implemented and looks reasonable to me.
> 
> We _may_ want to unify these two "hidestuff" into a list of patterns
> that hides any ref that match one of the patterns in the list, e.g.
> 
> 	set hidestuff {refs/heads/*/* refs/tags/* refs/remotes/*}
> 
> may hide all tags, all remote-tracking branches and local branches
> that have a slash in their names.
> 
> But that is an independent change that can come later.

This would make much more sense with gitk being abel to read a
per-repository configuration file, say from [file join $gitdir k] and
then save it there (but only if the file exists). I will send a separate
patch in a moment.

>>  gitk-git/gitk |   23 +++++++++++++++--------
>>  1 files changed, 15 insertions(+), 8 deletions(-)
>>
>> diff --git a/gitk-git/gitk b/gitk-git/gitk
>> index d93bd99..274b46b 100755
>> --- a/gitk-git/gitk
>> +++ b/gitk-git/gitk
>> @@ -1754,7 +1754,7 @@ proc readrefs {} {
>>      global tagids idtags headids idheads tagobjid
>>      global otherrefids idotherrefs mainhead mainheadid
>>      global selecthead selectheadid
>> -    global hideremotes
>> +    global hideremotes hidetags
>>  
>>      foreach v {tagids idtags headids idheads otherrefids idotherrefs} {
>>  	catch {unset $v}
>> @@ -1776,6 +1776,7 @@ proc readrefs {} {
>>  	    set headids($name) $id
>>  	    lappend idheads($id) $name
>>  	} elseif {[string match "tags/*" $name]} {
>> +	    if {$hidetags} continue
>>  	    # this lets refs/tags/foo^{} overwrite refs/tags/foo,
>>  	    # which is what we want since the former is the commit ID
>>  	    set name [string range $name 5 end]
>> @@ -2702,7 +2703,7 @@ proc savestuff {w} {
>>      global cmitmode wrapcomment datetimeformat limitdiffs
>>      global colors uicolor bgcolor fgcolor diffcolors diffcontext selectbgcolor
>>      global autoselect autosellen extdifftool perfile_attrs markbgcolor use_ttk
>> -    global hideremotes want_ttk
>> +    global hideremotes hidetags want_ttk
>>  
>>      if {$stuffsaved} return
>>      if {![winfo viewable .]} return
>> @@ -2725,6 +2726,7 @@ proc savestuff {w} {
>>  	puts $f [list set autosellen $autosellen]
>>  	puts $f [list set showneartags $showneartags]
>>  	puts $f [list set hideremotes $hideremotes]
>> +	puts $f [list set hidetags $hidetags]
>>  	puts $f [list set showlocalchanges $showlocalchanges]
>>  	puts $f [list set datetimeformat $datetimeformat]
>>  	puts $f [list set limitdiffs $limitdiffs]
>> @@ -10864,7 +10866,7 @@ proc create_prefs_page {w} {
>>  proc prefspage_general {notebook} {
>>      global NS maxwidth maxgraphpct showneartags showlocalchanges
>>      global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
>> -    global hideremotes want_ttk have_ttk
>> +    global hideremotes hidetags want_ttk have_ttk
>>  
>>      set page [create_prefs_page $notebook.general]
>>  
>> @@ -10887,6 +10889,9 @@ proc prefspage_general {notebook} {
>>      ${NS}::checkbutton $page.hideremotes -text [mc "Hide remote refs"] \
>>  	-variable hideremotes
>>      grid x $page.hideremotes -sticky w
>> +    ${NS}::checkbutton $page.hidetags -text [mc "Hide tag labels"] \
>> +	-variable hidetags
>> +    grid x $page.hidetags -sticky w
>>  
>>      ${NS}::label $page.ddisp -text [mc "Diff display options"]
>>      grid $page.ddisp - -sticky w -pady 10
>> @@ -10988,7 +10993,7 @@ proc doprefs {} {
>>      global oldprefs prefstop showneartags showlocalchanges
>>      global uicolor bgcolor fgcolor ctext diffcolors selectbgcolor markbgcolor
>>      global tabstop limitdiffs autoselect autosellen extdifftool perfile_attrs
>> -    global hideremotes want_ttk have_ttk
>> +    global hideremotes hidetags want_ttk have_ttk
>>  
>>      set top .gitkprefs
>>      set prefstop $top
>> @@ -10997,7 +11002,7 @@ proc doprefs {} {
>>  	return
>>      }
>>      foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
>> -		   limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
>> +		   limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
>>  	set oldprefs($v) [set $v]
>>      }
>>      ttk_toplevel $top
>> @@ -11117,7 +11122,7 @@ proc prefscan {} {
>>      global oldprefs prefstop
>>  
>>      foreach v {maxwidth maxgraphpct showneartags showlocalchanges \
>> -		   limitdiffs tabstop perfile_attrs hideremotes want_ttk} {
>> +		   limitdiffs tabstop perfile_attrs hideremotes hidetags want_ttk} {
>>  	global $v
>>  	set $v $oldprefs($v)
>>      }
>> @@ -11131,7 +11136,7 @@ proc prefsok {} {
>>      global oldprefs prefstop showneartags showlocalchanges
>>      global fontpref mainfont textfont uifont
>>      global limitdiffs treediffs perfile_attrs
>> -    global hideremotes
>> +    global hideremotes hidetags
>>  
>>      catch {destroy $prefstop}
>>      unset prefstop
>> @@ -11177,7 +11182,8 @@ proc prefsok {} {
>>  	  $limitdiffs != $oldprefs(limitdiffs)} {
>>  	reselectline
>>      }
>> -    if {$hideremotes != $oldprefs(hideremotes)} {
>> +    if {$hideremotes != $oldprefs(hideremotes) ||
>> +        $hidetags != $oldprefs(hidetags)} {
>>  	rereadrefs
>>      }
>>  }
>> @@ -11601,6 +11607,7 @@ set cmitmode "patch"
>>  set wrapcomment "none"
>>  set showneartags 1
>>  set hideremotes 0
>> +set hidetags 0
>>  set maxrefs 20
>>  set maxlinelen 200
>>  set showlocalchanges 1
> 

-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP