Re: [PATCH (GITK) 5/6] gitk: Fixed automatic row selection during load.
From: Paul Mackerras <hidden>
Date: 2016-06-15 22:45:05
Alexander Gavrilov writes:
- Switching views now actually preserves the selected commit. - Reloading (also Edit View) preserves the currently selected commit. - Initial selection does not produce weird scrolling. Signed-off-by: Alexander Gavrilov <redacted>
I need a more detailed explanation of the rationale for the specific changes you have made in the changelog. As for the patch, it mostly looks good, but I have a few comments below.
+proc getcommits {selid} {
global canv curview need_redisplay viewactive
initlayout
if {[start_rev_list $curview]} {
+ reset_pending_select $selidIs there any significance to having the call to reset_pending_select after the start_rev_list call (other than not setting pending_select if start_rev_list fails)? I couldn't see any. If there is then it should be noted in a comment and/or the patch description.
quoted hunk ↗ jump to hunk
@@ -503,7 +511,7 @@ proc updatecommits {} { filerun $fd [list getcommitlines $fd $i $view 1] incr viewactive($view) set viewcomplete($view) 0 - set pending_select $mainheadid + reset_pending_select {}
This doesn't actually change anything, right? If so then I'd prefer the simple, direct assignment to calling a procedure that does the assignment.
quoted hunk ↗ jump to hunk
@@ -4036,6 +4042,7 @@ proc layoutmore {} { } if {[info exists pending_select] && [commitinview $pending_select $curview]} { + update selectline [rowofcommit $pending_select] 1
What does that update do? Would update idletasks be better? Thanks, Paul.