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

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

[Łukasz Stelmach]

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(-)

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

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

[Junio C Hamano]

Ł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.

But that is an independent change that can come later.

>  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

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

[Lukasz Stelmach]

W dniu 02.12.2012 03:16, Junio C Hamano pisze:
> Ł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?

> 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

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

[Łukasz Stelmach]

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(-)

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

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

[Stefan Haller]

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/

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

[Lukasz Stelmach]

W dniu 03.12.2012 22:15, Stefan Haller pisze:
> 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.


> 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

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

[Łukasz Stelmach]

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(-)

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

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

[Marc Branchaud]

On 12-12-04 07:49 PM, Łukasz Stelmach 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.
> 
> 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.

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

[Łukasz Stelmach]

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(-)

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

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

[Junio C Hamano]

Ł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.  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.

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

[Paul Mackerras]

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).  But I think
it should be an opt-in thing.

> 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.

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

[Lukasz Stelmach]

W dniu 09.12.2012 11:44, Paul Mackerras pisze:
> 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?

>> 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

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

[Lukasz Stelmach]

W dniu 09.12.2012 10:18, Junio C Hamano pisze:
> Ł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...

> 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.

> 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).

> 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

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

[Felipe Contreras]

On Sat, Dec 1, 2012 at 8:16 PM, Junio C Hamano <gitster@pobox.com> wrote:

> 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

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

[Paul Mackerras]

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>
> > ---
> 
> 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.

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

[Junio C Hamano]

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.

Łukasz, what do you think?

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

[Lukasz Stelmach]

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.

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

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

[Junio C Hamano]

Lukasz Stelmach <stlman@poczta.fm> writes:

> 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.