Hi Rafael
Thanks for working on this
On 04/01/2021 16:21, Rafael Silva wrote:
"git worktree list" porcelain format shows one attribute per line, each
attribute is listed with a label and value separated by a single space.
If any worktree is locked, a "locked" attribute is listed followed by the
reason, if available, otherwise only "locked" is shown.
In case the lock reason contains newline characters (i.e: LF or CRLF)
this will cause the format to break as each line should correspond to
one attribute. This leads to unwanted behavior, specially as the
porcelain is intended to be machine-readable. To address this shortcoming
let's teach "git worktree list" to escape any newline character before
outputting the locked reason for porcelain format.
As the porcelain output format cannot handle worktree paths that contain
newlines either I think it would be better to add a `-z` flag which uses
NUL termination as we have for many other commands (diff, ls-files etc).
This would fix the problem forever without having to worry about quoting
each time a new field is added.
If you really need to quote the lock reason then please use one of the
path quoting routines (probably quote_c_style() or write_name_quoted())
in quote.c rather than rolling your own - the code below gives the same
output for a string that contains the two characters `\` followed by `n`
as it does for the single character `\n`. People are (hopefully) used to
dequoting our path quoting and there are routines out there to do it (we
have one git Git.pm) using a function in quote.c will allow them to use
those routines here.
Best Wishes
Phillip
Signed-off-by: Rafael Silva <rafaeloliveira.cs@xxxxxxxxx>
---
builtin/worktree.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index dedd4089e5..9156ccd67e 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -567,6 +567,24 @@ static int add(int ac, const char **av, const char *prefix)
return add_worktree(path, branch, &opts);
}
+static char *worktree_escape_reason(char *reason)
+{
+ struct strbuf escaped = STRBUF_INIT;
+ char *r;
+
+ for (r = reason; *r; r++) {
+ if (*r == '\r' && *(r + 1) && *(r + 1) == '\n') {
+ strbuf_addstr(&escaped, "\\r\\n");
+ r++;
+ } else if (*r == '\n')
+ strbuf_addstr(&escaped, "\\n");
+ else
+ strbuf_addch(&escaped, *r);
+ }
+
+ return strbuf_detach(&escaped, NULL);
+}
+
static void show_worktree_porcelain(struct worktree *wt)
{
printf("worktree %s\n", wt->path);
@@ -580,9 +598,11 @@ static void show_worktree_porcelain(struct worktree *wt)
printf("branch %s\n", wt->head_ref);
if (worktree_lock_reason(wt)) {
- if (*wt->lock_reason)
- printf("locked %s\n", wt->lock_reason);
- else
+ if (*wt->lock_reason) {
+ char *reason = worktree_escape_reason(wt->lock_reason);
+ printf("locked %s\n", reason);
+ free(reason);
+ } else
printf("locked\n");
}
