Thank you for all the feedback, I will work on a another patch that will address all the feedback provided on this series, including having the changes into a single patch. On Mon, Sep 28, 2020 at 02:19:53PM -0700, Junio C Hamano wrote: > Rafael Silva <rafaeloliveira.cs@xxxxxxxxx> writes: > > > This patch series introduces a new information on the git `worktree list` > > command output, to mark when a worktree is locked with a (locked) text mark. > > > > The intent is to improve the user experience to earlier sinalize that a linked > > worktree is locked, instead of realising later when attempting to remove it > > with `remove` command as it happened to me twice :) > > Change with a good intention, it seems. > > > The patches are divided into two parts. First part introduces > > the new marker to the worktree list command and small documentation > > change. And the second adds one test case into t2402 to test > > if the (locked) text will be properly set for a locked worktree, and > > not mistankely set to a unlocked or master worktree. > > Probably they belong together in a single patch (I am saying this > after only seeing the above five lines, without reading either of > these two patches, so there may be some things in them that makes it > make sense to have them separate). > Yes, I believe it make sense to have them in a single patch. > > This is the output of the worktree list with locked marker: > > > > $ git worktree list > > /repo/to/main abc123 [master] > > /path/to/unlocked-worktree1 456def [brancha] > > /path/to/locked-worktree 123abc (detached HEAD) (locked) > > Looks OK to me > > > This patches are marked with RFC mainly due to: > > > > - This will change the default behaviour of the worktree list, I am > > not sure whether will be better to make this tuned via a config > > and/or a git parameter. (assuming this change is a good idea ;) ) > > The default output is meant for human consumption (scripts that want > to read from the command and expect stable output would be using the > "--porcelain" option). > > The ideal case is a new output is universally useful for everybody, > in which case we can just change it without any new configuration or > command line option. Thanks for the explanation, will keep that in mind. > > > - Perhaps the `(locked)` marker is not the best suitable way to output > > this information and we might need to come with a better way. > > It looks good enough to me. I am not qualified to have a strong > opinion on this part, as I do not use the command all that often. > > $ git shortlog --no-merges --since=18.months builtin/worktree.c > > tells me that Eric Sunshine (CC'ed) may be a good source of wisdom > on this command. > Thank you, I will keep Eric Sunshine in the loop as well. > > - I am a new contributor to the code base, still learning a lot of git > > internals data structure and commands. Likely this patch will require > > updates. > > Welcome. Thank you. > > > Rafael Silva (2): > > teach `list` to mark locked worktree > > t2402: add test to locked linked worktree marker > > > > Documentation/git-worktree.txt | 5 +++-- > > builtin/worktree.c | 6 +++++- > > t/t2402-worktree-list.sh | 13 +++++++++++++ > > 3 files changed, 21 insertions(+), 3 deletions(-)
