Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-28 Thread Kaartic Sivaraam
On Tuesday 28 November 2017 09:34 AM, Junio C Hamano wrote: Eric Sunshine writes: With this approach, validate_worktree() will print an error message saying that the worktree directory is missing before the control info is actually removed. Kaartic's original patch

Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-27 Thread Eric Sunshine
On Mon, Nov 27, 2017 at 11:04 PM, Junio C Hamano wrote: > Eric Sunshine writes: >> I had envisioned a simple 'goto remove_control_info' rather than extra >> nesting or refactoring, but that's a minor point. > > Yes, use of goto is also a good way to

Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-27 Thread Junio C Hamano
Eric Sunshine writes: > With this approach, validate_worktree() will print an error message > saying that the worktree directory is missing before the control info > is actually removed. Kaartic's original patch silenced the message > (and _all_ error messages from

Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-27 Thread Eric Sunshine
On Mon, Nov 27, 2017 at 10:02 PM, Junio C Hamano wrote: > I actually wonder if this "early check and return" is making the > code unmaintainable. What if we instead did it with just the > codeflow restructuring, perhaps like so? > > if (!validate_worktree(wt, 0)) { >

Re: [RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-27 Thread Junio C Hamano
Kaartic Sivaraam writes: > diff --git a/builtin/worktree.c b/builtin/worktree.c > index b5afba164..6eab91889 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -605,6 +605,23 @@ static int move_worktree(int ac, const char **av, const > char *prefix) >

[RFC PATCH v2] builtin/worktree: enhance worktree removal

2017-11-27 Thread Kaartic Sivaraam
"git worktree remove" removes both the named worktree directory and the administrative information for it after checking that there is no local modifications that would be lost (which is a handy safety measure). However, due to a possible oversight, it aborts with an error if the worktree