On Fri, Feb 25, 2022 at 12:59 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > "Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> 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@xxxxxxxxxxxxxx/ [2]: https://lore.kernel.org/git/936f9b7c-6d54-00bc-f136-4cb4c2836eb6@xxxxxxxxx/ > > + 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.
