threads / patch / 32253

patchgitk: add a checkbox to control the visibility of tags

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

## tl;dr

18 messages between Nov 30, 2012 and Jan 2, 2013. Diffs are folded; open one to read it.

replies: 17people: 6as markdown or json

Łukasz Stelmach· Nov 30, 2012, 21:08 UTC · lore

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 <stlman@poczta.fm>
---
 gitk-git/gitk |   23 +++++++++++++++--------
 1 files changed, 15 insertions(+), 8 deletions(-)
Show changes to gitk-git/gitk +15 −8
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

-- 
1.7.8.6
Junio C Hamano· Dec 2, 2012, 02:16 UTC · re: Łukasz Stelmach · lore

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

Łukasz Stelmach <stlman@poczta.fm> writes:
Show 7 quoted lines
> 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 <stlman@poczta.fm>
> ---

Paul, this patch is not done against your tree (does not have gitk at the top-level), 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.
Show 114 quoted lines
>  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
Lukasz Stelmach· Dec 2, 2012, 21:25 UTC · re: Junio C Hamano · lore

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

W dniu 02.12.2012 03:16, Junio C Hamano pisze:
Show 12 quoted lines
> Łukasz Stelmach <stlman@poczta.fm> 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 <stlman@poczta.fm>
>> ---
> 
> 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?
Show 12 quoted lines
> 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.

Show 115 quoted lines
>>  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
Łukasz Stelmach· Dec 2, 2012, 21:29 UTC · re: Lukasz Stelmach · lore

[PATCH] gitk: read and write a repository specific configuration file

Enable gitk read and write repository specific configuration file: ".git/k" if the file exists. To make gitk use the local file simply create one, e.g. with the touch(1) command.

This is very useful if one uses different views for different repositories. Now there is no need to store all of them in ~/.gitk and make the views list needlesly long.

Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
 gitk-git/gitk |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)
Show changes to gitk-git/gitk +14 −11
diff --git a/gitk-git/gitk b/gitk-git/gitk
index d93bd99..60cf4cd 100755
--- a/gitk-git/gitk
+++ b/gitk-git/gitk
@@ -2696,7 +2696,7 @@ proc doprogupdate {} {
 
 proc savestuff {w} {
     global canv canv2 canv3 mainfont textfont uifont tabstop
-    global stuffsaved findmergefiles maxgraphpct
+    global stuffsaved findmergefiles maxgraphpct gitdir
     global maxwidth showneartags showlocalchanges
     global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment datetimeformat limitdiffs
@@ -2707,10 +2707,12 @@ proc savestuff {w} {
     if {$stuffsaved} return
     if {![winfo viewable .]} return
     catch {
-	if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
-	set f [open "~/.gitk-new" w]
+	set fn [expr [file exists [file join $gitdir k]] ? \
+		{[file join $gitdir k-new]} : {"~/.gitk-new"}]
+	if {[file exists $fn]} {file delete -force $fn}
+	set f [open $fn  w]
 	if {$::tcl_platform(platform) eq {windows}} {
-	    file attributes "~/.gitk-new" -hidden true
+	    catch {file attributes "~/.gitk-new" -hidden true}
 	}
 	puts $f [list set mainfont $mainfont]
 	puts $f [list set textfont $textfont]
@@ -2762,7 +2764,7 @@ proc savestuff {w} {
 	}
 	puts $f "}"
 	close $f
-	file rename -force "~/.gitk-new" "~/.gitk"
+	file rename -force $fn [regsub {\-new$} $fn {}]
     }
     set stuffsaved 1
 }
@@ -11663,7 +11665,14 @@ namespace import ::msgcat::mc
 ## And eventually load the actual message catalog
 ::msgcat::mcload $gitk_msgsdir
 
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+    show_error {} . [mc "Cannot find a git repository here."]
+    exit 1
+}
+
 catch {source ~/.gitk}
+catch {source [file join $gitdir k]}
 
 parsefont mainfont $mainfont
 eval font create mainfont [fontflags mainfont]
@@ -11680,12 +11689,6 @@ setui $uicolor
 
 setoptions
 
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-    show_error {} . [mc "Cannot find a git repository here."]
-    exit 1
-}
-
 set selecthead {}
 set selectheadid {}
 

-- 
1.7.8.6
Stefan Haller· Dec 3, 2012, 21:15 UTC · re: Łukasz Stelmach · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

Lukasz Stelmach <stlman@poczta.fm> wrote:
> Enable gitk read and write repository specific configuration
> file: ".git/k" if the file exists. To make gitk use the local
> file simply create one, e.g. with the touch(1) command.

I'm not sure I like this proposal. While it may be desirable to have *some* settings stored per repository, for most settings I want them to be remembered globally.

Git-gui tries to solve this by presenting two panes in the preferences dialog, so that I can choose the scope of every setting I change. This still doesn't help for things that are remembered implicitly, like the window size.

I don't have good suggestions how to solve this; just pointing out problems.

-- 
Stefan Haller
Berlin, Germany
http://www.haller-berlin.de/
Lukasz Stelmach· Dec 4, 2012, 20:12 UTC · re: Stefan Haller · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

W dniu 03.12.2012 22:15, Stefan Haller pisze:
Show 9 quoted lines
> Lukasz Stelmach <stlman@poczta.fm> wrote:
> 
>> Enable gitk read and write repository specific configuration
>> file: ".git/k" if the file exists. To make gitk use the local
>> file simply create one, e.g. with the touch(1) command.
> 
> I'm not sure I like this proposal. While it may be desirable to have
> *some* settings stored per repository, for most settings I want them to
> be remembered globally.

The way it works with my patch, gitk reads global settings from ~/.gitk. So you can treat it as a template. Then, when you exit it saves to local file if it exists. This of course means you can't override settings from ./.git/k with something from ~/.gitk by simply choosing to on GUI. However, it takes no more than removing appropriate line from .git/k to get the value from ~/.gitk.

IMHO this is a reasonable compromise which is available at no cost as far as data structure complexity is concerned. Choosing where to save what would require a bit of information per configuration variable. With a mask saved locally surprises may come when you change a variable, forget to localise it and it pops in a different repository. My approach, however simplistic, avoids this particular pitfall.

Show 9 quoted lines
> Git-gui tries to solve this by presenting two panes in the preferences
> dialog, so that I can choose the scope of every setting I change. This
> still doesn't help for things that are remembered implicitly, like the
> window size.
> 
> I don't have good suggestions how to solve this; just pointing out
> problems.
> 
> 
-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP
Łukasz Stelmach· Dec 5, 2012, 00:49 UTC · re: Łukasz Stelmach · lore

[PATCH v2] gitk: read and write a repository specific configuration file

Enable gitk read and write repository specific configuration file: ".git/k" if the file exists. To make gitk use the local file simply create one, e.g. with the touch(1) command.

This is very useful if one uses different views for different repositories. Now there is no need to store all of them in ~/.gitk and make the views list needlesly long.

Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
Same as before but rebased onto Paul's repository.
 gitk |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)
Show changes to gitk +14 −11
diff --git a/gitk b/gitk
index 379582a..c6b7dc3 100755
--- a/gitk
+++ b/gitk
@@ -2703,7 +2703,7 @@ proc doprogupdate {} {
 
 proc savestuff {w} {
     global canv canv2 canv3 mainfont textfont uifont tabstop
-    global stuffsaved findmergefiles maxgraphpct
+    global stuffsaved findmergefiles maxgraphpct gitdir
     global maxwidth showneartags showlocalchanges
     global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment datetimeformat limitdiffs
@@ -2714,10 +2714,12 @@ proc savestuff {w} {
     if {$stuffsaved} return
     if {![winfo viewable .]} return
     catch {
-	if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
-	set f [open "~/.gitk-new" w]
+	set fn [expr [file exists [file join $gitdir k]] ? \
+		{[file join $gitdir k-new]} : {"~/.gitk-new"}]
+	if {[file exists $fn]} {file delete -force $fn}
+	set f [open $fn  w]
 	if {$::tcl_platform(platform) eq {windows}} {
-	    file attributes "~/.gitk-new" -hidden true
+	    catch {file attributes "~/.gitk-new" -hidden true}
 	}
 	puts $f [list set mainfont $mainfont]
 	puts $f [list set textfont $textfont]
@@ -2769,7 +2771,7 @@ proc savestuff {w} {
 	}
 	puts $f "}"
 	close $f
-	file rename -force "~/.gitk-new" "~/.gitk"
+	file rename -force $fn [regsub {\-new$} $fn {}]
     }
     set stuffsaved 1
 }
@@ -11723,7 +11725,14 @@ namespace import ::msgcat::mc
 ## And eventually load the actual message catalog
 ::msgcat::mcload $gitk_msgsdir
 
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+    show_error {} . [mc "Cannot find a git repository here."]
+    exit 1
+}
+
 catch {source ~/.gitk}
+catch {source [file join $gitdir k]}
 
 parsefont mainfont $mainfont
 eval font create mainfont [fontflags mainfont]
@@ -11740,12 +11749,6 @@ setui $uicolor
 
 setoptions
 
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-    show_error {} . [mc "Cannot find a git repository here."]
-    exit 1
-}
-
 set selecthead {}
 set selectheadid {}
 

-- 
1.7.8.6
Marc Branchaud· Dec 5, 2012, 15:20 UTC · re: Łukasz Stelmach · lore

Re: [PATCH v2] gitk: read and write a repository specific configuration file

On 12-12-04 07:49 PM, Łukasz Stelmach wrote:
Show 7 quoted lines
> Enable gitk read and write repository specific configuration
> file: ".git/k" if the file exists. To make gitk use the local
> file simply create one, e.g. with the touch(1) command.
> 
> This is very useful if one uses different views for different
> repositories. Now there is no need to store all of them in
> ~/.gitk and make the views list needlesly long.
s/needlesly/needlessly/
		M.
Łukasz Stelmach· Dec 8, 2012, 11:27 UTC · re: Marc Branchaud · lore

[PATCH] gitk: read and write a repository specific configuration file

Enable gitk read and write repository specific configuration file: ".git/k" if the file exists. To make gitk use the local file simply create one, e.g. with the touch(1) command.

This is very useful if one uses different views for different repositories. Now there is no need to store all of them in ~/.gitk and make the views list needlessly long.

Signed-off-by: Łukasz Stelmach <stlman@poczta.fm>
---
 gitk |   25 ++++++++++++++-----------
 1 files changed, 14 insertions(+), 11 deletions(-)
Show changes to gitk +14 −11
diff --git a/gitk b/gitk
index 379582a..c6b7dc3 100755
--- a/gitk
+++ b/gitk
@@ -2703,7 +2703,7 @@ proc doprogupdate {} {
 
 proc savestuff {w} {
     global canv canv2 canv3 mainfont textfont uifont tabstop
-    global stuffsaved findmergefiles maxgraphpct
+    global stuffsaved findmergefiles maxgraphpct gitdir
     global maxwidth showneartags showlocalchanges
     global viewname viewfiles viewargs viewargscmd viewperm nextviewnum
     global cmitmode wrapcomment datetimeformat limitdiffs
@@ -2714,10 +2714,12 @@ proc savestuff {w} {
     if {$stuffsaved} return
     if {![winfo viewable .]} return
     catch {
-	if {[file exists ~/.gitk-new]} {file delete -force ~/.gitk-new}
-	set f [open "~/.gitk-new" w]
+	set fn [expr [file exists [file join $gitdir k]] ? \
+		{[file join $gitdir k-new]} : {"~/.gitk-new"}]
+	if {[file exists $fn]} {file delete -force $fn}
+	set f [open $fn  w]
 	if {$::tcl_platform(platform) eq {windows}} {
-	    file attributes "~/.gitk-new" -hidden true
+	    catch {file attributes "~/.gitk-new" -hidden true}
 	}
 	puts $f [list set mainfont $mainfont]
 	puts $f [list set textfont $textfont]
@@ -2769,7 +2771,7 @@ proc savestuff {w} {
 	}
 	puts $f "}"
 	close $f
-	file rename -force "~/.gitk-new" "~/.gitk"
+	file rename -force $fn [regsub {\-new$} $fn {}]
     }
     set stuffsaved 1
 }
@@ -11723,7 +11725,14 @@ namespace import ::msgcat::mc
 ## And eventually load the actual message catalog
 ::msgcat::mcload $gitk_msgsdir
 
+# check that we can find a .git directory somewhere...
+if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
+    show_error {} . [mc "Cannot find a git repository here."]
+    exit 1
+}
+
 catch {source ~/.gitk}
+catch {source [file join $gitdir k]}
 
 parsefont mainfont $mainfont
 eval font create mainfont [fontflags mainfont]
@@ -11740,12 +11749,6 @@ setui $uicolor
 
 setoptions
 
-# check that we can find a .git directory somewhere...
-if {[catch {set gitdir [exec git rev-parse --git-dir]}]} {
-    show_error {} . [mc "Cannot find a git repository here."]
-    exit 1
-}
-
 set selecthead {}
 set selectheadid {}
 

-- 
1.7.8.6
Junio C Hamano· Dec 9, 2012, 09:18 UTC · re: Łukasz Stelmach · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

Łukasz Stelmach <stlman@poczta.fm> writes:
Show 7 quoted lines
> Enable gitk read and write repository specific configuration
> file: ".git/k" if the file exists. To make gitk use the local
> file simply create one, e.g. with the touch(1) command.
>
> This is very useful if one uses different views for different
> repositories. Now there is no need to store all of them in
> ~/.gitk and make the views list needlessly long.

I do not use gitk heavily myself, but I have a mixed feeling about this patch.

Forking the configuration from the one true ~/.gitk is easy; it is just the matter of copying it to repository specific location. Once forked, however, it is very hard to merge these configuration files sprinkled across repositories back, or more importantly, change the settings globally. Imagine you just got a new monitor that is a lot finer grained than the one you have been usingq, and your choice of font size has been specified in terms of pixels; you would want to show all gitk windows in larger font now, regardless of the repository, but you now have to go to 47 different configuration files and update them.

So I suspect that this may introduce more trouble than it is worth for users and should not be sold with a "This is very useful" label. At best, it is "This may be useful"; otherwise the feature may end up harming our users. I'd phrase it without judging if it is good or bad for the users, perhaps like this:

    This allows one to specify different views for different
    repositories.

In any case, the filename .git/k may be _cute_, but I do not think we would want to see:

    $ ls .git
    branches        config       HEAD   index  k     objects
    COMMIT_EDITMSG  description  hooks  info   logs  refs

It is too cryptic, unless the user _knows_ 'k' is for gitk. I'd call it $GIT_DIR/gitkconfig or something, if I were supportive for this feature (which I am not enthusiastic, yet).

Thanks.
Paul Mackerras· Dec 9, 2012, 10:44 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

On Sun, Dec 09, 2012 at 01:18:08AM -0800, Junio C Hamano wrote:
Show 12 quoted lines
> Łukasz Stelmach <stlman@poczta.fm> writes:
> 
> > Enable gitk read and write repository specific configuration
> > file: ".git/k" if the file exists. To make gitk use the local
> > file simply create one, e.g. with the touch(1) command.
> >
> > This is very useful if one uses different views for different
> > repositories. Now there is no need to store all of them in
> > ~/.gitk and make the views list needlessly long.
> 
> I do not use gitk heavily myself, but I have a mixed feeling about
> this patch.

I agree, I think this would be surprising to people who are used to the way gitk works now.

I could imagine having a checkbox in the Edit->Preferences dialog to say "Save configuration settings locally", and if you check that box, then it writes the configuration to .git/gitkconfig or whatever (having first saved that setting in the global ~/.gitk). But I think it should be an opt-in thing.

Show 10 quoted lines
> In any case, the filename .git/k may be _cute_, but I do not think
> we would want to see:
> 
>     $ ls .git
>     branches        config       HEAD   index  k     objects
>     COMMIT_EDITMSG  description  hooks  info   logs  refs
> 
> It is too cryptic, unless the user _knows_ 'k' is for gitk.  I'd
> call it $GIT_DIR/gitkconfig or something, if I were supportive for
> this feature (which I am not enthusiastic, yet).
I agree with this too.
Paul.
Lukasz Stelmach· Dec 9, 2012, 13:25 UTC · re: Paul Mackerras · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

W dniu 09.12.2012 11:44, Paul Mackerras pisze:
Show 21 quoted lines
> On Sun, Dec 09, 2012 at 01:18:08AM -0800, Junio C Hamano wrote:
>> Łukasz Stelmach <stlman@poczta.fm> writes:
>>
>>> Enable gitk read and write repository specific configuration
>>> file: ".git/k" if the file exists. To make gitk use the local
>>> file simply create one, e.g. with the touch(1) command.
>>>
>>> This is very useful if one uses different views for different
>>> repositories. Now there is no need to store all of them in
>>> ~/.gitk and make the views list needlessly long.
>>
>> I do not use gitk heavily myself, but I have a mixed feeling about
>> this patch.
> 
> I agree, I think this would be surprising to people who are used to
> the way gitk works now.
>
> I could imagine having a checkbox in the Edit->Preferences dialog to
> say "Save configuration settings locally", and if you check that box,
> then it writes the configuration to .git/gitkconfig or whatever
> (having first saved that setting in the global ~/.gitk).

No this isn't a good idea. When you choose to save configuration locally it means you've alredy changed it to match your local needs and making it global does not seem reasonable.

> But I think it should be an opt-in thing.

It is opt-in now definitely. One needs to create the local config file, even an empty one, for gitk to choose it upon doquit/savestuff. I agree a checkbox may be more convenient but is it "opty" (as in opt-in) enough?

Then, the checkbox should be added to both "Preferences" and "Edit View" dialogs, anything more?

Show 12 quoted lines
>> In any case, the filename .git/k may be _cute_, but I do not think
>> we would want to see:
>>
>>     $ ls .git
>>     branches        config       HEAD   index  k     objects
>>     COMMIT_EDITMSG  description  hooks  info   logs  refs
>>
>> It is too cryptic, unless the user _knows_ 'k' is for gitk.  I'd
>> call it $GIT_DIR/gitkconfig or something, if I were supportive for
>> this feature (which I am not enthusiastic, yet).
> 
> I agree with this too.
Sure, let's vote:

a) .git/gitk b) .git/gitkconfig c) .git/gitkrc

-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP
Lukasz Stelmach· Dec 9, 2012, 13:13 UTC · re: Junio C Hamano · lore

Re: [PATCH] gitk: read and write a repository specific configuration file

W dniu 09.12.2012 10:18, Junio C Hamano pisze:
Show 18 quoted lines
> Łukasz Stelmach <stlman@poczta.fm> writes:
> 
>> Enable gitk read and write repository specific configuration
>> file: ".git/k" if the file exists. To make gitk use the local
>> file simply create one, e.g. with the touch(1) command.
>>
>> This is very useful if one uses different views for different
>> repositories. Now there is no need to store all of them in
>> ~/.gitk and make the views list needlessly long.
> 
> I do not use gitk heavily myself, but I have a mixed feeling about
> this patch.
> 
> Forking the configuration from the one true ~/.gitk is easy; it is
> just the matter of copying it to repository specific location.  Once
> forked, however, it is very hard to merge these configuration files
> sprinkled across repositories back, or more importantly, change the
> settings globally.

For the record, I assumed someone using git is capable of doing some simple tricks with find, sed and the like.

Merging configuration from the global file (~/.gitk) is quite easy as the file is sourced just before the local file. If any option is not set in the local file the global value is effective.

To handle the case you describe below...
Show 6 quoted lines
> Imagine you just got a new monitor that is a lot
> finer grained than the one you have been usingq, and your choice of
> font size has been specified in terms of pixels; you would want to
> show all gitk windows in larger font now, regardless of the
> repository, but you now have to go to 47 different configuration
> files and update them.
you need to (assume one keeps git repositoris below $HOME)
1. Enter a random repository
2. mv .git/gitk .git/gitk-local (see below)
3. Run gitk, configure fonts to your taste, save config (it will be
saved  globally)
4. mv .git/gitk-local .git/gitk
4 Do a trick
$ find ../ -name gitk -type f -path '*/.git/gitk' -print0 | \
  xargs -0 sed -i -e '/^set [a-z]\+font /d'

Now the font settings from ~/.gitk will be applied (and saved locally when gitk exits) in every repository find(1) found.

> So I suspect that this may introduce more trouble than it is worth
> for users and should not be sold with a "This is very useful" label.
> At best, it is "This may be useful";

I work with more than two dozen different repositories and saving the list of branches I want to see upon startup is quite important for me.

Show 6 quoted lines
> otherwise the feature may end
> up harming our users.  I'd phrase it without judging if it is good
> or bad for the users, perhaps like this:
> 
>     This allows one to specify different views for different
>     repositories.

At present the code won't harm anyone not willing to get harmed. To make gitk save the configuration locally user needs to create the configuration file manually, outside of gitk, for example with touch(1) (yes it may be empty).

Show 6 quoted lines
> In any case, the filename .git/k may be _cute_, but I do not think
> we would want to see:
> 
>     $ ls .git
>     branches        config       HEAD   index  k     objects
>     COMMIT_EDITMSG  description  hooks  info   logs  refs
I agree this was just to draw your attention ;-)
> It is too cryptic, unless the user _knows_ 'k' is for gitk.  I'd
> call it $GIT_DIR/gitkconfig or something, if I were supportive for
> this feature (which I am not enthusiastic, yet).
I think simply $GIT_DIR/gitk as in ~/.gitk is going to be fine.
-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP
Felipe Contreras· Dec 2, 2012, 21:40 UTC · re: Junio C Hamano · lore

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

On Sat, Dec 1, 2012 at 8:16 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> 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.
And hide the rest. Currently gitk shows all other refs (e.g. refs/hg/*).
-- 
Felipe Contreras
Paul Mackerras· Jan 2, 2013, 07:17 UTC · re: Junio C Hamano · lore

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

On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
Show 21 quoted lines
> Łukasz Stelmach <stlman@poczta.fm> 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 <stlman@poczta.fm>
> > ---
> 
> Paul, this patch is not done against your tree (does not have gitk
> at the top-level), 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.

If the concern is the amount of screen real-estate that the tags take up when there are many of them (which is a reasonable concern), I'd rather just put a single tag icon with "tags..." inside it and arrange to list all the tags in the diff display pane when the user clicks on it. I think that would be better than not showing the tags at all.

Paul.
Junio C Hamano· Jan 2, 2013, 07:24 UTC · re: Paul Mackerras · lore

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

Paul Mackerras <paulus@samba.org> writes:
Show 16 quoted lines
> On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
>> Łukasz Stelmach <stlman@poczta.fm> 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 <stlman@poczta.fm>
>> > ---
>> ... 
> If the concern is the amount of screen real-estate that the tags take
> up when there are many of them (which is a reasonable concern), I'd
> rather just put a single tag icon with "tags..." inside it and arrange
> to list all the tags in the diff display pane when the user clicks on
> it.  I think that would be better than not showing the tags at all.
Yeah, sounds very sensible.  Thanks.
Łukasz, what do you think?
Lukasz Stelmach· Jan 2, 2013, 08:03 UTC · re: Junio C Hamano · lore

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

W dniu 02.01.2013 08:24, Junio C Hamano pisze:
Show 20 quoted lines
> Paul Mackerras <paulus@samba.org> writes:
> 
>> On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
>>> Łukasz Stelmach <stlman@poczta.fm> 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 <stlman@poczta.fm>
>>>> ---
>>> ... 
>> If the concern is the amount of screen real-estate that the tags take
>> up when there are many of them (which is a reasonable concern), I'd
>> rather just put a single tag icon with "tags..." inside it and arrange
>> to list all the tags in the diff display pane when the user clicks on
>> it.  I think that would be better than not showing the tags at all.
> 
> Yeah, sounds very sensible.  Thanks.

I am afraid I don't really understand why tags should be listed in the diff pane only after clicking the "tags" tag (if this is what Junio has suggested)? How about just putting there another line saying: Tags, next to Parent and Chindren and all the stuff?

If something should happen upon user interaction with the tag label a toolpit would be a better choince FWIW.

-- 
Było mi bardzo miło.               Czwarta pospolita klęska, [...]
>Łukasz<                 Już nie katolicka lecz złodziejska.  (c)PP
Junio C Hamano· Jan 2, 2013, 17:08 UTC · re: Lukasz Stelmach · lore

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

Lukasz Stelmach <stlman@poczta.fm> writes:
Show 29 quoted lines
> W dniu 02.01.2013 08:24, Junio C Hamano pisze:
>> Paul Mackerras <paulus@samba.org> writes:
>> 
>>> On Sat, Dec 01, 2012 at 06:16:25PM -0800, Junio C Hamano wrote:
>>>> Łukasz Stelmach <stlman@poczta.fm> 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 <stlman@poczta.fm>
>>>>> ---
>>>> ... 
>>> If the concern is the amount of screen real-estate that the tags take
>>> up when there are many of them (which is a reasonable concern), I'd
>>> rather just put a single tag icon with "tags..." inside it and arrange
>>> to list all the tags in the diff display pane when the user clicks on
>>> it.  I think that would be better than not showing the tags at all.
>> 
>> Yeah, sounds very sensible.  Thanks.
>
> I am afraid I don't really understand why tags should be listed in the
> diff pane only after clicking the "tags" tag (if this is what Junio has
> suggested)? How about just putting there another line saying: Tags, next
> to Parent and Chindren and all the stuff?
>
> If something should happen upon user interaction with the tag label a
> toolpit would be a better choince FWIW.

If you meant tooltip that shows extra information in a pop-up when the user hovers the pointer, I think that would be a workable solution, too, and probably is a better one, compared to showing it in another pane that may or may not be selected.

← back to recent threads