Message ID | pull.1023.git.1629807526939.gitgitgadget@gmail.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | gitk: new option to hide prefetch refs | expand |
On 8/24/2021 8:18 AM, Tal Kelrich via GitGitGadget wrote: > From: Tal Kelrich <hasturkun@gmail.com> > > The maintenance 'prefetch' task creates refs that mirror remote refs, > and in repositories with many branches this can clutter the commit list. > > Add a new option to ignore any prefetch refs, enabled by default. This seems like a sensible feature to add. Thank you for contributing! > It might have been better to allow gitk to read log.excludeDecoration > (or a gitk specific configured pattern) and use that to ignore refs, but > this was simpler for me to implement without knowing Tcl. I agree, but I also don't know Tcl so can't comment to how hard that might be. I'm not sure how gitk operates and why it does not automatically benefit from log.excludeDecoration. Is it computing its own decoration? Is it using a different Git command that isn't integrated with log.excludeDecoration, but should be? I also cannot review that this patch works as advertised. I just wanted to chime in with support for the idea. Thanks, -Stolee
Derrick Stolee <stolee@gmail.com> writes: > On 8/24/2021 8:18 AM, Tal Kelrich via GitGitGadget wrote: >> From: Tal Kelrich <hasturkun@gmail.com> >> >> The maintenance 'prefetch' task creates refs that mirror remote refs, >> and in repositories with many branches this can clutter the commit list. >> >> Add a new option to ignore any prefetch refs, enabled by default. > > This seems like a sensible feature to add. Thank you for contributing! > >> It might have been better to allow gitk to read log.excludeDecoration >> (or a gitk specific configured pattern) and use that to ignore refs, but >> this was simpler for me to implement without knowing Tcl. > > I agree, but I also don't know Tcl so can't comment to how hard that > might be. I'm not sure how gitk operates and why it does not automatically > benefit from log.excludeDecoration. Is it computing its own decoration? Is > it using a different Git command that isn't integrated with > log.excludeDecoration, but should be? > > I also cannot review that this patch works as advertised. I just wanted to > chime in with support for the idea. I would usually say that defaulting this to 'on' would be a biased choice [*1*], but in this case I tend to think it is a good idea to hide these by default, as 'prefetch' came way after people started using Git, and users did not ask for 'prefetch' refs. The prefetching may help users but the refs used to anchor the prefetched objects are implementation detail that the users would rather not to see. I wonder if we should also hide refs/stash for the same reason, but that is outside the scope of this change. In any case, please make it a patch relative to Paul's tree and send it in his direction. Thanks. [Footnote] *1* Whenever the inventor of a feature says "I expect users would want this!", it needs to be taken with a moderate amount of salt, as the inventor is self selected specimen who wanted it (after all, the feature motivated the inventor enough to write the patch).
On 8/24/2021 2:46 PM, Junio C Hamano wrote: > Derrick Stolee <stolee@gmail.com> writes: > >> On 8/24/2021 8:18 AM, Tal Kelrich via GitGitGadget wrote: >>> From: Tal Kelrich <hasturkun@gmail.com> >>> >>> The maintenance 'prefetch' task creates refs that mirror remote refs, >>> and in repositories with many branches this can clutter the commit list. >>> >>> Add a new option to ignore any prefetch refs, enabled by default. >> >> This seems like a sensible feature to add. Thank you for contributing! ...> I would usually say that defaulting this to 'on' would be a biased > choice [*1*], but in this case I tend to think it is a good idea to > hide these by default, as 'prefetch' came way after people started > using Git, and users did not ask for 'prefetch' refs. The prefetching > may help users but the refs used to anchor the prefetched objects are > implementation detail that the users would rather not to see. ... > *1* Whenever the inventor of a feature says "I expect users would > want this!", it needs to be taken with a moderate amount of salt, as > the inventor is self selected specimen who wanted it (after all, the > feature motivated the inventor enough to write the patch). I agree with both points. As the inventor of prefetch refs, my opinion should also be taken with similar salt. I do want to add at least that Git itself hides these refs by modifying log.excludeDecoration during 'git maintenance start', so there is precedent. Git is hiding them, but user can become confused when they appear in gitk. Thanks, -Stolee
diff --git a/gitk-git/gitk b/gitk-git/gitk index 23d9dd1fe0d..4e44da517f2 100755 --- a/gitk-git/gitk +++ b/gitk-git/gitk @@ -1780,6 +1780,7 @@ proc readrefs {} { global selecthead selectheadid global hideremotes global tclencoding + global hideprefetch foreach v {tagids idtags headids idheads otherrefids idotherrefs} { unset -nocomplain $v @@ -1814,6 +1815,7 @@ proc readrefs {} { } set tagids($name) $id lappend idtags($id) $name + } elseif {[string match "prefetch/*" $name] && $hideprefetch} { } else { set otherrefids($name) $id lappend idotherrefs($id) $name @@ -11548,7 +11550,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 maxrefs web_browser + global hideremotes want_ttk have_ttk maxrefs web_browser hideprefetch set page [create_prefs_page $notebook.general] @@ -11572,6 +11574,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.hideprefetch -text [mc "Hide prefetch refs"] \ + -variable hideprefetch + grid x $page.hideprefetch -sticky w ${NS}::label $page.ddisp -text [mc "Diff display options"] grid $page.ddisp - -sticky w -pady 10 @@ -11696,7 +11701,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 want_ttk have_ttk hideprefetch set top .gitkprefs set prefstop $top @@ -11705,7 +11710,8 @@ proc doprefs {} { return } foreach v {maxwidth maxgraphpct showneartags showlocalchanges \ - limitdiffs tabstop perfile_attrs hideremotes want_ttk} { + limitdiffs tabstop perfile_attrs hideremotes want_ttk \ + hideprefetch} { set oldprefs($v) [set $v] } ttk_toplevel $top @@ -11831,7 +11837,8 @@ proc prefscan {} { global oldprefs prefstop foreach v {maxwidth maxgraphpct showneartags showlocalchanges \ - limitdiffs tabstop perfile_attrs hideremotes want_ttk} { + limitdiffs tabstop perfile_attrs hideremotes want_ttk \ + hideprefetch} { global $v set $v $oldprefs($v) } @@ -11845,7 +11852,7 @@ proc prefsok {} { global oldprefs prefstop showneartags showlocalchanges global fontpref mainfont textfont uifont global limitdiffs treediffs perfile_attrs - global hideremotes + global hideremotes hideprefetch catch {destroy $prefstop} unset prefstop @@ -11891,7 +11898,8 @@ proc prefsok {} { $limitdiffs != $oldprefs(limitdiffs)} { reselectline } - if {$hideremotes != $oldprefs(hideremotes)} { + if {$hideremotes != $oldprefs(hideremotes) || + $hideprefetch != $oldprefs(hideprefetch)} { rereadrefs } } @@ -12365,6 +12373,7 @@ set cmitmode "patch" set wrapcomment "none" set showneartags 1 set hideremotes 0 +set hideprefetch 1 set maxrefs 20 set visiblerefs {"master"} set maxlinelen 200 @@ -12477,7 +12486,7 @@ set config_variables { filesepbgcolor filesepfgcolor linehoverbgcolor linehoverfgcolor linehoveroutlinecolor mainheadcirclecolor workingfilescirclecolor indexcirclecolor circlecolors linkfgcolor circleoutlinecolor diffbgcolors - web_browser + web_browser hideprefetch } foreach var $config_variables { config_init_trace $var