Re: [PATCH v2 0/2] git-gui: revert untracked files by deleting them

[Pratyush Yadav <me@yadavpratyush.com>]

On 07/11/19 07:05AM, Jonathan Gilbert via GitGitGadget wrote:
> My development environment sometimes makes automatic changes that I don't
> want to keep. In some cases, this involves new files being added that I
> don't want to commit or keep (but I also don't want to outright .gitignore 
> forever). I have typically had to explicitly delete those files externally
> to Git Gui, which is a context switch to a manual operation, and I want to
> be able to just select those newly-created untracked files in the UI and
> "revert" them into oblivion.
> 
> This change updates the revert_helper proc to check for untracked files as
> well as changes, and then changes to be reverted and untracked files are
> handled by independent blocks of code. The user is prompted independently
> for untracked files, since the underlying action is fundamentally different
> (rm -f). If after deleting untracked files, the directory containing them
> becomes empty, then the directory is removed as well. A new proc 
> delete_files takes care of actually deleting the files, using the Tcler's
> Wiki recommended approach for keeping the UI responsive.
> 
> Since the checkout_index and delete_files calls are both asynchronous and
> could potentially complete in any order, a "chord" is used to coordinate
> unlocking the index and returning the UI to a usable state only after both
> operations are complete.
> 
> This is the third revision of this change, which differs from the second
> version in the following ways:
> 
>  * A new construct called a "chord" is used to coordinate the completion of
>    multiple asynchronous operations that can be kicked off by revert_helper.
>    A chord is, conceptually, a procedure with multiple entrypoints whose
>    body only executes once all entrypoints have been activated. The 
>    chord.tcl file includes comprehensive documentation of how to use the
>    chord classes.
>    
>    
>  * Since we might not yet be ready to unlock the index when checkout_index 
>    returns, the _close_updateindex proc where it was ultimately unlocking
>    the index has been modified so that unlocking the index is the
>    responsibility of the caller. Since the $after functionality ran after 
>    unlock_index, that is also hoisted out. Nothing in _close_updateindex 
>    appears to be asynchronous, so the caller can simply make the calls
>    itself upon its return.

The cover letter is so much more descriptive than the commit message. It 
would be nice to have all this context and commentary in the commit 
message. Of course, you'd remove the "personal workflow" bit and some 
other stuff, but most of this can be copied verbatim.

Also, like I mentioned in the review of your second patch, 
`_close_updateindex` _does_ have an asynchronous component 
unfortunately.
    
>    
>  * lexists has been renamed to path_exists.
>    
>    
>  * Up to 10 deletion errors are now shown simultaneously. I also confirmed
>    that Tcl's file delete code will always return a nicely-formatted error
>    including the filename, and changed the message so that it isn't also 
>    injecting the filename.

-- 
Regards,
Pratyush Yadav