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 <angavrilov@xxxxxxxxx> 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 $selid Is 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. > @@ -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. > @@ -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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html