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