Thread (83 messages) 83 messages, 3 authors, 2021-01-30

Re: [PATCH 0/7] teach `worktree list` verbose mode and prunable annotations

flat view

From: Rafael Silva <hidden>
Date: 2021-01-08 07:39:22

Eric Sunshine writes:
On Mon, Jan 4, 2021 at 11:22 AM Rafael Silva
[off-list ref] wrote:
quoted
In c57b3367be (worktree: teach `list` to annotate locked worktree,
2020-10-11) we taught `git worktree list` to annotate working tree that
is locked by appending "locked" text in order to signalize to the user
that a working tree is locked.  During the review, there was some
discussion about additional annotations and information that `list`
command could provide to the user that has long been envisioned and
mentioned in [2], [3] and [4].

This patch series address some of these changes by teaching
`worktree list` to show "prunable" annotation, adding verbose mode and
extending the --porcelain format with prunable and locked annotation as
follow up from [1]. Additionally, it address one shortcoming for porcelain
format to escape any newline characters (LF and CRLF) for the lock reason
to prevent breaking format mentioned in [4] and [1] during the review
cycle.
Thank you for working on this. I'm happy to see these long-envisioned
enhancements finally taking shape. Before even reviewing the patches,
I decided to apply them and play with the new features, and I'm very
pleased to see that they behave exactly as I had envisioned all those
years ago.

Very nicely done.
Thank you. I'm glad to hear the patches are aligned with what you
envisioned.
I'll review the patches when I finish responding to this cover letter.
Thank you for reviewing and applying the patches, really appreciate it.
quoted
The fifth patch adds worktree_escape_reason() that accepts a (char *)
text and returned the text with any LF or CRLF escaped. The caller is
responsible to freeing the escaped text. This is used by the locked
annotation in porcelain format. Currently, this is defined within
builtin/worktree.c as I was not sure whether libfying the function as
part of this series is a good idea. At this time it seems more sensible
to leave the code internally and libfying later once we are confident
about the implementation and whether it can be used in other part of the
code base but I'm open for suggestion.
Perhaps I misunderstand, but I had envisioned employing one of the
codebase's existing quoting/escaping functions rather than crafting a
new one from scratch. However, I'll reserve judgment until I actually
read the patch.
Agreed. It make sense to reuse one of the already implemented functions
from the code base. for some reason I was not able to find it. I believe
this was cleared out in one of the patches replies by you and Phillip Wood.

I will remove this and reuse one of the existing function on the next
revision.

-- 
Thanks
Rafael
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help