From: Jonathan Gilbert <JonathanG@xxxxxxxxxxxx>
Update the revert_helper procedure to also detect untracked files. If
files are present, the user is asked if they want them deleted. Perform
the deletion in batches, using new proc delete_files with helper
delete_helper, to allow the UI to remain responsive. Coordinate the
completion of multiple overlapping asynchronous operations using a new
construct called a "chord". Migrate unlocking of the index out of
_close_updateindex to a responsibility of the caller, to permit paths
that don't directly unlock the index.
Signed-off-by: Jonathan Gilbert <JonathanG@xxxxxxxxxxxx>
---
lib/chord.tcl | 137 ++++++++++++++++++++++++
lib/index.tcl | 288 +++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 385 insertions(+), 40 deletions(-)
create mode 100644 lib/chord.tcl
diff --git a/lib/chord.tcl b/lib/chord.tcl
new file mode 100644
index 0000000000..2d13af14fc
--- /dev/null
+++ b/lib/chord.tcl
@@ -0,0 +1,137 @@
+# SimpleChord class:
+# Represents a procedure that conceptually has multiple entrypoints that must
+# all be called before the procedure executes. Each entrypoint is called a
+# "note". The chord is only "completed" when all the notes are "activated".
+#
+# Constructor:
+# set chord [SimpleChord new {body}]
+# Creates a new chord object with the specified body script. The body
+# script is evaluated at most once, when a note is activated and the
+# chord has no other non-activated notes.
+#
+# Methods:
+# $chord eval {script}
+# Runs the specified script in the same context (namespace) in which the
+# chord body will be evaluated. This can be used to set variable values
+# for the chord body to use.
+#
+# set note [$chord add_note]
+# Adds a new note to the chord, an instance of ChordNote. Raises an
+# error if the chord is already completed, otherwise the chord is updated
+# so that the new note must also be activated before the body is
+# evaluated.
+#
+# $chord notify_note_activation
+# For internal use only.
+#
+# ChordNote class:
+# Represents a note within a chord, providing a way to activate it. When the
+# final note of the chord is activated (this can be any note in the chord,
+# with all other notes already previously activated in any order), the chord's
+# body is evaluated.
+#
+# Constructor:
+# Instances of ChordNote are created internally by calling add_note on
+# SimpleChord objects.
+#
+# Methods:
+# [$note is_activated]
+# Returns true if this note has already been activated.
+#
+# $note
+# Activates the note, if it has not already been activated, and completes
+# the chord if there are no other notes awaiting activation. Subsequent
+# calls will have no further effect.
+#
+# Example:
+#
+# # Turn off the UI while running a couple of async operations.
+# lock_ui
+#
+# set chord [SimpleChord new {
+# unlock_ui
+# # Note: $notice here is not referenced in the calling scope
+# if {$notice} { info_popup $notice }
+# }
+#
+# # Configure a note to keep the chord from completing until
+# # all operations have been initiated.
+# set common_note [$chord add_note]
+#
+# # Pass notes as 'after' callbacks to other operations
+# async_operation $args [$chord add_note]
+# other_async_operation $args [$chord add_note]
+#
+# # Communicate with the chord body
+# if {$condition} {
+# # This sets $notice in the same context that the chord body runs in.
+# $chord eval { set notice "Something interesting" }
+# }
+#
+# # Activate the common note, making the chord eligible to complete
+# $common_note
+#
+# At this point, the chord will complete at some unknown point in the future.
+# The common note might have been the first note activated, or the async
+# operations might have completed synchronously and the common note is the
+# last one, completing the chord before this code finishes, or anything in
+# between. The purpose of the chord is to not have to worry about the order.
+
+oo::class create SimpleChord {
+ variable Notes
+ variable Body
+ variable IsCompleted
+
+ constructor {body} {
+ set Notes [list]
+ set Body $body
+ set IsCompleted 0
+ }
+
+ method eval {script} {
+ namespace eval [self] $script
+ }
+
+ method add_note {} {
+ if {$IsCompleted} { error "Cannot add a note to a completed chord" }
+
+ set note [ChordNote new [self]]
+
+ lappend Notes $note
+
+ return $note
+ }
+
+ method notify_note_activation {} {
+ if {!$IsCompleted} {
+ foreach note $Notes {
+ if {![$note is_activated]} { return }
+ }
+
+ set IsCompleted 1
+
+ namespace eval [self] $Body
+ namespace delete [self]
+ }
+ }
+}
+
+oo::class create ChordNote {
+ variable Chord IsActivated
+
+ constructor {chord} {
+ set Chord $chord
+ set IsActivated 0
+ }
+
+ method is_activated {} {
+ return $IsActivated
+ }
+
+ method unknown {} {
+ if {!$IsActivated} {
+ set IsActivated 1
+ $Chord notify_note_activation
+ }
+ }
+}
diff --git a/lib/index.tcl b/lib/index.tcl
index 28d4d2a54e..64046d6833 100644
--- a/lib/index.tcl
+++ b/lib/index.tcl
@@ -7,7 +7,7 @@ proc _delete_indexlock {} {
}
}
-proc _close_updateindex {fd after} {
+proc _close_updateindex {fd} {
global use_ttk NS
fconfigure $fd -blocking 1
if {[catch {close $fd} err]} {
@@ -52,8 +52,6 @@ proc _close_updateindex {fd after} {
}
$::main_status stop
- unlock_index
- uplevel #0 $after
}
proc update_indexinfo {msg path_list after} {
@@ -90,7 +88,9 @@ proc write_update_indexinfo {fd path_list total_cnt batch after} {
global file_states current_diff_path
if {$update_index_cp >= $total_cnt} {
- _close_updateindex $fd $after
+ _close_updateindex $fd
+ unlock_index
+ uplevel #0 $after
return
}
@@ -156,7 +156,9 @@ proc write_update_index {fd path_list total_cnt batch after} {
global file_states current_diff_path
if {$update_index_cp >= $total_cnt} {
- _close_updateindex $fd $after
+ _close_updateindex $fd
+ unlock_index
+ uplevel #0 $after
return
}
@@ -233,7 +235,8 @@ proc write_checkout_index {fd path_list total_cnt batch after} {
global file_states current_diff_path
if {$update_index_cp >= $total_cnt} {
- _close_updateindex $fd $after
+ _close_updateindex $fd $do_unlock_index $after
+ uplevel #0 $after
return
}
@@ -393,61 +396,266 @@ proc revert_helper {txt paths} {
if {![lock_index begin-update]} return
+ # Common "after" functionality that waits until multiple asynchronous
+ # operations are complete (by waiting for them to activate their notes
+ # on the chord).
+ set after_chord [SimpleChord new {
+ unlock_index
+ if {$should_reshow_diff} { reshow_diff }
+ ui_ready
+ }]
+
+ $after_chord eval { set should_reshow_diff 0 }
+
+ # We don't know how many notes we're going to create (it's dynamic based
+ # on conditional paths below), so create a common note that will delay
+ # the chord's completion until we activate it, and then activate it
+ # after all the other notes have been created.
+ set after_common_note [$after_chord add_note]
+
set path_list [list]
+ set untracked_list [list]
set after {}
foreach path $paths {
switch -glob -- [lindex $file_states($path) 0] {
U? {continue}
+ ?O {
+ lappend untracked_list $path
+ }
?M -
?T -
?D {
lappend path_list $path
if {$path eq $current_diff_path} {
- set after {reshow_diff;}
+ $after_chord eval { set should_reshow_diff 1 }
}
}
}
}
+ set path_cnt [llength $path_list]
+ set untracked_cnt [llength $untracked_list]
- # Split question between singular and plural cases, because
- # such distinction is needed in some languages. Previously, the
- # code used "Revert changes in" for both, but that can't work
- # in languages where 'in' must be combined with word from
- # rest of string (in different way for both cases of course).
- #
- # FIXME: Unfortunately, even that isn't enough in some languages
- # as they have quite complex plural-form rules. Unfortunately,
- # msgcat doesn't seem to support that kind of string translation.
- #
- set n [llength $path_list]
- if {$n == 0} {
- unlock_index
- return
- } elseif {$n == 1} {
- set query [mc "Revert changes in file %s?" [short_path [lindex $path_list]]]
- } else {
- set query [mc "Revert changes in these %i files?" $n]
- }
+ if {$path_cnt > 0} {
+ # Split question between singular and plural cases, because
+ # such distinction is needed in some languages. Previously, the
+ # code used "Revert changes in" for both, but that can't work
+ # in languages where 'in' must be combined with word from
+ # rest of string (in different way for both cases of course).
+ #
+ # FIXME: Unfortunately, even that isn't enough in some languages
+ # as they have quite complex plural-form rules. Unfortunately,
+ # msgcat doesn't seem to support that kind of string
+ # translation.
+ #
+ if {$path_cnt == 1} {
+ set query [mc \
+ "Revert changes in file %s?" \
+ [short_path [lindex $path_list]] \
+ ]
+ } else {
+ set query [mc \
+ "Revert changes in these %i files?" \
+ $path_cnt]
+ }
- set reply [tk_dialog \
- .confirm_revert \
- "[appname] ([reponame])" \
- "$query
+ set reply [tk_dialog \
+ .confirm_revert \
+ "[appname] ([reponame])" \
+ "$query
[mc "Any unstaged changes will be permanently lost by the revert."]" \
- question \
- 1 \
- [mc "Do Nothing"] \
- [mc "Revert Changes"] \
- ]
- if {$reply == 1} {
- checkout_index \
- $txt \
+ question \
+ 1 \
+ [mc "Do Nothing"] \
+ [mc "Revert Changes"] \
+ ]
+
+ if {$reply == 1} {
+ checkout_index \
+ $txt \
+ $path_list \
+ [$after_chord add_note]
+ }
+ }
+
+ if {$untracked_cnt > 0} {
+ # Split question between singular and plural cases, because
+ # such distinction is needed in some languages.
+ #
+ # FIXME: Unfortunately, even that isn't enough in some languages
+ # as they have quite complex plural-form rules. Unfortunately,
+ # msgcat doesn't seem to support that kind of string
+ # translation.
+ #
+ if {$untracked_cnt == 1} {
+ set query [mc \
+ "Delete untracked file %s?" \
+ [short_path [lindex $untracked_list]] \
+ ]
+ } else {
+ set query [mc \
+ "Delete these %i untracked files?" \
+ $untracked_cnt \
+ ]
+ }
+
+ set reply [tk_dialog \
+ .confirm_revert \
+ "[appname] ([reponame])" \
+ "$query
+
+[mc "Files will be permanently deleted."]" \
+ question \
+ 1 \
+ [mc "Do Nothing"] \
+ [mc "Delete Files"] \
+ ]
+
+ if {$reply == 1} {
+ $after_chord eval { set should_reshow_diff 1 }
+
+ delete_files $untracked_list [$after_chord add_note]
+ }
+ }
+
+ # Activate the common note. If no other notes were created, this
+ # completes the chord. If other notes were created, then this common
+ # note prevents a race condition where the chord might complete early.
+ $after_common_note
+}
+
+# Delete all of the specified files, performing deletion in batches to allow the
+# UI to remain responsive and updated.
+proc delete_files {path_list after} {
+ # Enable progress bar status updates
+ $::main_status start [mc "Deleting"] [mc "files"]
+
+ set path_index 0
+ set deletion_errors [list]
+ set batch_size 50
+
+ delete_helper \
+ $path_list \
+ $path_index \
+ $deletion_errors \
+ $batch_size \
+ $after
+}
+
+# Helper function to delete a list of files in batches. Each call deletes one
+# batch of files, and then schedules a call for the next batch after any UI
+# messages have been processed.
+proc delete_helper {path_list path_index deletion_errors batch_size after} {
+ global file_states
+
+ set path_cnt [llength $path_list]
+
+ set batch_remaining $batch_size
+
+ while {$batch_remaining > 0} {
+ if {$path_index >= $path_cnt} { break }
+
+ set path [lindex $path_list $path_index]
+
+ set deletion_failed [catch {file delete -- $path} deletion_error]
+
+ if {$deletion_failed} {
+ lappend deletion_errors [list "$deletion_error"]
+ } else {
+ remove_empty_directories [file dirname $path]
+
+ # Don't assume the deletion worked. Remove the file from
+ # the UI, but only if it no longer exists.
+ if {![path_exists $path]} {
+ unset file_states($path)
+ display_file $path __
+ }
+ }
+
+ incr path_index 1
+ incr batch_remaining -1
+ }
+
+ # Update the progress bar to indicate that this batch has been
+ # completed. The update will be visible when this procedure returns
+ # and allows the UI thread to process messages.
+ $::main_status update $path_index $path_cnt
+
+ if {$path_index < $path_cnt} {
+ # The Tcler's Wiki lists this as the best practice for keeping
+ # a UI active and processing messages during a long-running
+ # operation.
+
+ after idle [list after 0 [list \
+ delete_helper \
$path_list \
- [concat $after [list ui_ready]]
+ $path_index \
+ $deletion_errors \
+ $batch_size \
+ $after
+ ]]
} else {
- unlock_index
+ # Finish the status bar operation.
+ $::main_status stop
+
+ # Report error, if any, based on how many deletions failed.
+ set deletion_error_cnt [llength $deletion_errors]
+
+ if {($deletion_error_cnt > 0) && ($deletion_error_cnt <= [MAX_VERBOSE_FILES_IN_DELETION_ERROR])} {
+ set error_text "Encountered errors deleting files:\n"
+
+ foreach deletion_error $deletion_errors {
+ append error_text "* [lindex $deletion_error 0]\n"
+ }
+
+ error_popup $error_text
+ } elseif {$deletion_error_cnt == $path_cnt} {
+ error_popup [mc \
+ "None of the %d selected files could be deleted." \
+ $path_cnt \
+ ]
+ } elseif {$deletion_error_cnt > 1} {
+ error_popup [mc \
+ "%d of the %d selected files could not be deleted." \
+ $deletion_error_cnt \
+ $path_cnt \
+ ]
+ }
+
+ uplevel #0 $after
+ }
+}
+
+proc MAX_VERBOSE_FILES_IN_DELETION_ERROR {} { return 10; }
+
+# This function is from the TCL documentation:
+#
+# https://wiki.tcl-lang.org/page/file+exists
+#
+# [file exists] returns false if the path does exist but is a symlink to a path
+# that doesn't exist. This proc returns true if the path exists, regardless of
+# whether it is a symlink and whether it is broken.
+proc path_exists {name} {
+ expr {![catch {file lstat $name finfo}]}
+}
+
+# Remove as many empty directories as we can starting at the specified path,
+# walking up the directory tree. If we encounter a directory that is not
+# empty, or if a directory deletion fails, then we stop the operation and
+# return to the caller. Even if this procedure fails to delete any
+# directories at all, it does not report failure.
+proc remove_empty_directories {directory_path} {
+ set parent_path [file dirname $directory_path]
+
+ while {$parent_path != $directory_path} {
+ set contents [glob -nocomplain -dir $directory_path *]
+
+ if {[llength $contents] > 0} { break }
+ if {[catch {file delete -- $directory_path}]} { break }
+
+ set directory_path $parent_path
+ set parent_path [file dirname $directory_path]
}
}
--
gitgitgadget
