From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
This series includes three patches that I sent 8 days ago,
because I haven't received any reply so far.
These patches address the following problems:
1) Runaway processes (resend)
As in the 'git gui blame' case, gitk back-end processes can sometimes
run for a while without producing any output, e.g. diff-files on a slow
filesystem.
These patches make gitk explicitly kill its back-end processes.
2) Gitk stopping showing any diffs under random conditions.
3) Broken commit selection on view reload: gitk tried to preserve
the selected commit, but usually failed because of code rot.
I added selection preservation on Reload and Edit View.
Update still should reset the selection to HEAD if anything changed.
Also, if the previously selected commit was not found in the new view,
gitk should fall back to selecting HEAD.
Alexander Gavrilov (6):
gitk: Kill back-end processes on window close.
gitk: Register diff-files & diff-index in commfd, to ensure kill.
gitk: On Windows use a Cygwin-specific flag for kill.
gitk: Fixed broken exception handling in diff.
gitk: Fixed automatic row selection during load.
gitk: Fallback to selecting the head commit upon load.
gitk | 148 +++++++++++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 97 insertions(+), 51 deletions(-)
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
Date: Sat, 12 Jul 2008 16:09:28 +0400
When collecting commits for a rarely changed, or recently
created file or directory, rev-list may work for a noticeable
period of time without producing any output. Such processes
don't receive SIGPIPE for a while after gitk is closed, thus
becoming runaway CPU hogs.
Signed-off-by: Alexander Gavrilov <redacted>
---
gitk | 35 +++++++++++++++++++++++++----------
1 files changed, 25 insertions(+), 10 deletions(-)
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
Date: Sun, 13 Jul 2008 16:40:47 +0400
Local change analysis can take a noticeable amount of time on large
file sets, and produce no output if there are no changes. Register
the back-ends in commfd, so that they get properly killed on window
close.
Signed-off-by: Alexander Gavrilov <redacted>
---
gitk | 39 +++++++++++++++++++++++----------------
1 files changed, 23 insertions(+), 16 deletions(-)
@@ -90,6 +90,15 @@ proc dorunq {} { } }+proc reg_instance {fd} {+ global commfd leftover loginstance++ set i [incr loginstance]+ set commfd($i) $fd+ set leftover($i) {}+ return $i+}+ proc unmerged_files {files} { global nr_unmerged
@@ -294,10 +303,10 @@ proc parseviewrevs {view revs} { # Start off a git log process and arrange to read its output proc start_rev_list {view} { global startmsecs commitidx viewcomplete curview- global commfd leftover tclencoding+ global tclencoding global viewargs viewargscmd viewfiles vfilelimit global showlocalchanges commitinterest- global viewactive loginstance viewinstances vmergeonly+ global viewactive viewinstances vmergeonly global pending_select mainheadid global vcanopt vflags vrevs vorigargs
@@ -354,10 +363,8 @@ proc start_rev_list {view} { error_popup "[mc "Error executing git log:"] $err" return 0 }- set i [incr loginstance]+ set i [reg_instance $fd] set viewinstances($view) [list $i]- set commfd($i) $fd- set leftover($i) {} if {$showlocalchanges && $mainheadid ne {}} { lappend commitinterest($mainheadid) {dodiffindex} }
@@ -420,8 +427,8 @@ proc getcommits {} { proc updatecommits {} { global curview vcanopt vorigargs vfilelimit viewinstances- global viewactive viewcomplete loginstance tclencoding- global startmsecs commfd showneartags showlocalchanges leftover+ global viewactive viewcomplete tclencoding+ global startmsecs showneartags showlocalchanges global mainheadid pending_select global isworktree global varcid vposids vnegids vflags vrevs
@@ -482,10 +489,8 @@ proc updatecommits {} { if {$viewactive($view) == 0} { set startmsecs [clock clicks -milliseconds] }- set i [incr loginstance]+ set i [reg_instance $fd] lappend viewinstances($view) $i- set commfd($i) $fd- set leftover($i) {} fconfigure $fd -blocking 0 -translation lf -eofchar {} if {$tclencoding != {}} { fconfigure $fd -encoding $tclencoding
@@ -4063,10 +4068,11 @@ proc dodiffindex {} { incr lserial set fd [open "|git diff-index --cached HEAD" r] fconfigure $fd -blocking 0- filerun $fd [list readdiffindex $fd $lserial]+ set i [reg_instance $fd]+ filerun $fd [list readdiffindex $fd $lserial $i] }-proc readdiffindex {fd serial} {+proc readdiffindex {fd serial inst} { global mainheadid nullid nullid2 curview commitinfo commitdata lserial set isdiff 1
@@ -4077,7 +4083,7 @@ proc readdiffindex {fd serial} { set isdiff 0 } # we only need to see one line and we don't really care what it says...- close $fd+ stop_instance $inst if {$serial != $lserial} { return 0
@@ -4086,7 +4092,8 @@ proc readdiffindex {fd serial} { # now see if there are any local changes not checked in to the index set fd [open "|git diff-files" r] fconfigure $fd -blocking 0- filerun $fd [list readdifffiles $fd $serial]+ set i [reg_instance $fd]+ filerun $fd [list readdifffiles $fd $serial $i] if {$isdiff && ![commitinview $nullid2 $curview]} { # add the line for the changes in the index to the graph
@@ -4103,7 +4110,7 @@ proc readdiffindex {fd serial} { return 0 }-proc readdifffiles {fd serial} {+proc readdifffiles {fd serial inst} { global mainheadid nullid nullid2 curview global commitinfo commitdata lserial
@@ -4115,7 +4122,7 @@ proc readdifffiles {fd serial} { set isdiff 0 } # we only need to see one line and we don't really care what it says...- close $fd+ stop_instance $inst if {$serial != $lserial} { return 0
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
Date: Tue, 15 Jul 2008 00:35:42 +0400
MSysGit compiles git binaries as native Windows executables,
so they cannot be killed unless a special flag is specified.
This flag is implemented by the Cygwin version of kill,
which is also included in MSysGit.
Signed-off-by: Alexander Gavrilov <redacted>
---
gitk | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
Date: Sat, 26 Jul 2008 18:48:41 +0400
If the tree diff command failed to start for some
random reason, treepending remained set, and thus
no more diffs were shown after that.
Signed-off-by: Alexander Gavrilov <redacted>
---
gitk | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
@@ -307,7 +307,7 @@ proc start_rev_list {view} { global viewargs viewargscmd viewfiles vfilelimit global showlocalchanges commitinterest global viewactive viewinstances vmergeonly- global pending_select mainheadid+ global mainheadid global vcanopt vflags vrevs vorigargs set startmsecs [clock clicks -milliseconds]
@@ -374,9 +374,6 @@ proc start_rev_list {view} { } filerun $fd [list getcommitlines $fd $i $view 0] nowbusy $view [mc "Reading"]- if {$view == $curview} {- set pending_select $mainheadid- } set viewcomplete($view) 0 set viewactive($view) 1 return 1
@@ -418,11 +415,22 @@ proc stop_rev_list {view} { set viewinstances($view) {} }-proc getcommits {} {+proc reset_pending_select {selid} {+ global pending_select mainheadid++ if {$selid ne {}} {+ set pending_select $selid+ } else {+ set pending_select $mainheadid+ }+}++proc getcommits {selid} { global canv curview need_redisplay viewactive initlayout if {[start_rev_list $curview]} {+ reset_pending_select $selid show_status [mc "Reading commits..."] set need_redisplay 1 } else {
@@ -515,6 +523,11 @@ proc reloadcommits {} { global showneartags treediffs commitinterest cached_commitrow global targetid+ set selid {}+ if {$selectedline ne {}} {+ set selid $currentid+ }+ if {!$viewcomplete($curview)} { stop_rev_list $curview }
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:03
Date: Sat, 26 Jul 2008 20:15:54 +0400
Try selecting the head, if the previously selected commit
is not available in the new view.
Signed-off-by: Alexander Gavrilov <redacted>
---
gitk | 25 ++++++++++++++++++-------
1 files changed, 18 insertions(+), 7 deletions(-)
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 $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.
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:05
On Thu, Jul 31, 2008 at 3:25 PM, Paul Mackerras [off-list ref] wrote:
Alexander Gavrilov writes:
quoted
- 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.
The rationale is that preserving the currently selected commit is more
intelligent behavior than always resetting to a preset position, and
it makes the UI feel more smooth. Also, although it is possible to
restore selection by clicking the 'back' button, it is not immediately
obvious; in fact I realized it only after reading the code.
Gitk already tried to preserve the commit in many cases, but failed
because of a conflicting change that made it select the head. Such
behavior is clearly a bug.
The Reload case is arguable, but I think that Edit View should
preserve the selection, and it uses Reload internally. It can be
resolved by adding a parameter to the function implementing Reload.
As for the patch, it mostly looks good, but I have a few comments
below.
quoted
+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.
It simply tries to preserve the original behavior.
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.
I have a patch WIP that allows specifying a commit on the command line
to select instead of the head (I need it to enhance the git gui blame
UI). It makes the function somewhat more intelligent. I'll submit it
as soon as this series is sorted out.
What does that update do? Would update idletasks be better?
That update forces Tk to recompute the widget dimensions. Otherwise
selectline sometimes gets all zeroes from yview, which makes it
compute really weird scrolling settings. Git-gui always does an update
before scrolling computations.
Alexander
From: Alexander Gavrilov <hidden> Date: 2016-06-15 22:45:06
Date: Sun, 27 Jul 2008 08:18:27 +0400
Other GUI tools may occasionally need to start
gitk and make it automatically select a certain
commit. This patch supports doing it through the
environment or command line.
Using the environment allows graceful degradation of
the tool when used with an old version of gitk:
unsupported command line options cause it to die.
Signed-off-by: Alexander Gavrilov <redacted>
---
On Thursday 31 July 2008 16:41:20 Alexander Gavrilov wrote:
> I have a patch WIP that allows specifying a commit on the command line
> to select instead of the head (I need it to enhance the git gui blame
> UI). It makes the function somewhat more intelligent. I'll submit it
> as soon as this series is sorted out.
I decided to send it now.
-- Alexander
gitk | 25 ++++++++++++++++++++++++-
1 files changed, 24 insertions(+), 1 deletions(-)
@@ -416,10 +416,12 @@ proc stop_rev_list {view} { } proc reset_pending_select {selid} {- global pending_select mainheadid+ global pending_select mainheadid selectheadid if {$selid ne {}} { set pending_select $selid+ } elseif {$selectheadid ne {}} {+ set pending_select $selectheadid } else { set pending_select $mainheadid }
@@ -1607,6 +1609,7 @@ proc getcommit {id} { proc readrefs {} { global tagids idtags headids idheads tagobjid global otherrefids idotherrefs mainhead mainheadid+ global selecthead selectheadid foreach v {tagids idtags headids idheads otherrefids idotherrefs} { catch {unset $v}
@@ -1653,6 +1656,12 @@ proc readrefs {} { set mainhead [string range $thehead 11 end] } }+ set selectheadid {}+ if {$selecthead ne {}} {+ catch {+ set selectheadid [exec git rev-parse --verify $selecthead]+ }+ } } # skip over fake commits
@@ -9863,6 +9872,13 @@ if {![file isdirectory $gitdir]} { exit 1 }+set selecthead {}+set selectheadid {}++if {[info exists env(GITK_SELECT_ID)]} {+ set selecthead $env(GITK_SELECT_ID)+}+ set revtreeargs {} set cmdline_files {} set i 0
@@ -9874,6 +9890,9 @@ foreach arg $argv { set cmdline_files [lrange $argv [expr {$i + 1}] end] break }+ "--select-commit=*" {+ set selecthead [string range $arg 16 end]+ } "--argscmd=*" { set revtreeargscmd [string range $arg 10 end] }
@@ -9884,6 +9903,10 @@ foreach arg $argv { incr i }+if {$selecthead eq "HEAD"} {+ set selecthead {}+}+ if {$i >= [llength $argv] && $revtreeargs ne {}} { # no -- on command line, but some arguments (other than --argscmd) if {[catch {