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

On Fri, Jan 8, 2021 at 5:33 AM Phillip Wood <phillip.wood123@gmail.com> wrote:

Backward compatibility is a valid concern, though I haven't managed to convince myself that it would matter much in this case. In one sense, the failure of the porcelain format to properly quote/escape paths when needed can be viewed as an outright bug and, although we value backward compatibility, we also value correctness, and such bug fixes have been accepted in the past. Especially in a case such as this, it seems exceedingly unlikely that fixing the bug would be harmful or break existing tooling (though, of course that possibility always exists, even if remotely so).

I can imagine ways in which tooling might be engineered to work around the shortcoming that `git worktree list --porcelain` doesn't properly handle newlines embedded in paths, but such tooling would almost certainly be so fragile anyhow that it would break as we add more keys to the extensible porcelain format. Coupled with the fact that newlines embedded in paths are so exceedingly unlikely, it's hard to imagine that fixing this bug would have a big impact on existing tooling.

The case you mention about a path which happens to have a double-quote as its first character does concern me a bit more since existing tooling wouldn't have had to jump through hoops, or indeed do anything special, with such paths, unlike the embedded newline case. But then, too, it's pretty hard to imagine this coming up much, if at all, in practice. That's not to say that I can't imagine a path, in general, beginning with a quote, but keeping in mind that we're talking only about worktree paths, it seems exceedingly unlikely.

My gut feeling (for what it's worth) is that worktree paths containing embedded newlines (or other special characters) or beginning with a double-quote is so unlikely to come in in actual practice that viewing this as a bug fix is probably a reasonable approach, whereas some other approach -- such as introducing porcelain-v2 or creating a new porcelain key, say "worktreepath" which supersedes "worktree" (without retiring "worktree") -- may be overkill.

None of the above is an argument against a complementary `-z` mode, which I think is a very good idea.

Rafael's patch is quoting only the lock-reason, not the worktree path, so I think it's orthogonal to this discussion. Also, his patch is introducing `lock` as a new attribute in porcelain output, not modifying behavior of an existing `lock` attribute.