From: Philip Oakley Date: Thu, 17 Dec 2015 22:10:45 GMT Subject: Re: [PATCH v2 4/4] git gui: allow for a long recentrepo list Message-ID: <2F62ABA2EE734F5F926C9E102969895F@PhilipOakley> In-Reply-To: <1CAD7B4439DF4AE19EE8CD56CB47F399@PhilipOakley> From: "Philip Oakley" > From: "Junio C Hamano" , December 17, 2015 7:32 PM >> Philip Oakley writes: >> >>> The gui.recentrepo list may be longer than the maxrecent setting. >>> Allow extra space to show any extra entries. >>> >>> In an ideal world, the git gui would limit the number of entries >>> to the maxrecent setting, however the recentrepo config list may >>> have been extended outside of the gui, or the maxrecent setting changed >>> to a reduced value. Further, when testing the gui's recentrepo >>> logic it is useful to show these extra, but valid, entries. >> >> I do not have a strong objection either way, but the magic number 5 >> smells like an indication that this is solving a wrong problem or >> solving a problem in a wrong approach. What happens to the entries >> beyond $maxrecent-th in the recent list with the current code? The >> list effectively truncated and not shown? Wouldn't the same thing >> happen to the entries beyond ($maxrecent+5)-th in the recent list >> with your patch? > > True, it was a magic number selected as being a moderate overspill > allowance. > I tried 3 and 5 as possible values, and 5 didn't look ungainly, even on my > small netbook, and covered all my test hacking cases. > > The issue was that the prior code assumed that the fault could never > happen and limited the display, which is sorted, to the max value, so in a > fault condition the most recent repo could fall off the bottom. > > As a bit of 'monkey patching' I wasn't sure how the dialog creation could > know in advance how long the list would be, so I simply added on a small > margin. > >> >> Truncating the list at $maxrecent-th to match the display >> (i.e. declare that "$maxrecent" is really the max number of entries >> we would keep track of), or allowing the UI to scroll so that >> entries beyond $maxrecent-th one can be shown, perhaps? > > This would certainly be a reasonable approach, but I don't know enough of > the tcl to know how to make it happen. If anyone has suggestions... I think I see the way to do it. V3 Patch 4/4 to follow. > >> >>> >>> Signed-off-by: Philip Oakley >>> --- >>> word usage corrected. >>> Eric's comments $gmane/282432 >>> --- >>> git-gui/lib/choose_repository.tcl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/git-gui/lib/choose_repository.tcl >>> b/git-gui/lib/choose_repository.tcl >>> index ad7a888..a08ed4f 100644 >>> --- a/git-gui/lib/choose_repository.tcl >>> +++ b/git-gui/lib/choose_repository.tcl >>> @@ -153,7 +153,7 @@ constructor pick {} { >>> -background [get_bg_color $w_body.recentlabel] \ >>> -wrap none \ >>> -width 50 \ >>> - -height $maxrecent >>> + -height [expr {$maxrecent + 5}] >>> $w_recentlist tag conf link \ >>> -foreground blue \ >>> -underline 1 >> -- >> To unsubscribe from this list: send the line "unsubscribe git" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> > > -- > To unsubscribe from this list: send the line "unsubscribe git" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >