Re: [PATCH] worktree: add -z option for list subcommand

[Eric Sunshine <sunshine@sunshineco.com>]

On Fri, Feb 25, 2022 at 12:59 PM Junio C Hamano <gitster@pobox.com> wrote:
> "Phillip Wood via GitGitGadget" <gitgitgadget@gmail.com> writes:
> > +-z::
> > +     When `--porcelain` is specified with `list` terminate each line with a
> > +     NUL rather than a newline. This makes it possible to parse the output
> > +     when a worktree path contains a newline character.
>
> This makes it sound as if it were impossible.  As the changes this
> patch makes to the documentation and the code indicate, we were
> already doing quote_c_style(), so it is not quite correct to say so.
>
> Perhaps "makes it easier" is more accurate?
>
> Unless the original wasn't using quote_c_style() correctly and
> wasn't quoting its output, that is?

That's the case. It is impossible without this patch since `git
worktree list --porcelain` does not call quote_c_style() for the
worktree path; it only calls quote_c_style() for the lock reason.
Fixing this oversight for the worktree path for non-`-z` mode has
potential backward-compatibility concerns. I had argued[1] that we
might be able to sell it as a pure bug fix, but Phillip was
concerned[2] that it might break existing tooling. (I also share that
concern, but to a lesser degree, perhaps, since worktrees with oddball
names containing newline or a leading double-quote must be exceedingly
rare.)

[1]: https://lore.kernel.org/git/CAPig+cQq_RnanDQ3jHfNz_L58WyzmsUJBhtdrLxa=H0v_io+WA@mail.gmail.com/
[2]: https://lore.kernel.org/git/936f9b7c-6d54-00bc-f136-4cb4c2836eb6@gmail.com/

> > +             if (line_terminator) {
> > +                     quote_c_style(reason, &sb, NULL, 0);
> > +                     reason = sb.buf;
> > +             }
> > +             printf("locked %s%c", reason, line_terminator);
>
> OK.  I suspect write_name_quoted() may be a better fit that does not
> require us to have our own strbuf, but this should be OK.

Nice suggestion.