Re: [PATCH 6/7] worktree remove: new command

On Fri, Feb 2, 2018 at 6:47 PM, Eric Sunshine [off-list ref] wrote:

On Wed, Jan 24, 2018 at 4:53 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:

quoted

This command allows to delete a worktree. Like 'move' you cannot
remove the main worktree, or one with submodules inside [1].
[...]
Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---

diff --git a/builtin/worktree.c b/builtin/worktree.c

@@ -688,6 +689,132 @@ static int move_worktree(int ac, const char **av, const char *prefix)
+static void check_clean_worktree(struct worktree *wt,
+                                const char *original_path)
+{
+       [...]
+       validate_no_submodules(wt);

It's slightly strange seeing worktree validation in a function
checking whether the worktree is clean since submodule validation
isn't an issue of cleanliness. I'd have expected the caller to invoke
it instead:

    int remove_worktree(...) {
        ...
        if (!force) {
            validate_no_submodules(wt);
            check_clean_worktree(wt, av[0]);
        }
        ...
    }

On the other hand, I could imagine it being called here as appropriate
if submodule validation eventually also checks submodule cleanliness
as hinted by the commit message.

Yes I basically consider all submodules dirty until somebody sorts
them out. I'll add a comment here to clarify this.
-- 
Duy