On 28/02/2022 08:35, Eric Sunshine wrote:
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.)
If the user has not set core.quotePath to false then any unicode paths
will be quoted unless we only quote paths with newlines and leading
double quotes which would mean the quoting would not match all the other
git commands. In general I think it is easier to use nul termination
rather than having to unquote path names. In this case it is not obvious
to me what command one would feed a quoted directory name into either
(it's not like doing "git diff | sed ... |git update-index --stdin" or
something like that).
Best Wishes
Phillip
[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.
