Re: [PATCH v2 4/6] worktree: teach `list --porcelain` to annotate locked worktree

On Tue, Jan 19, 2021 at 3:20 AM Rafael Silva
[off-list ref] wrote:

Eric Sunshine writes:

quoted

On Sun, Jan 17, 2021 at 6:43 PM Rafael Silva
[off-list ref] wrote:

quoted

+               quote_c_style(reason, &sb, NULL, CQUOTE_NODQ);

This needs a change, and it's totally my fault that it does. In my
previous review, I mentioned that if the lock reason contains special
characters, we want those special characters escaped and the reason
quoted, but _only_ if it contains special characters. However, I then
incorrectly said to call quote_c_style() with CQUOTE_NODQ to achieve
that behavior. In fact, CQUOTE_NODQ gives us the wrong behavior since
it avoids quoting the string which, as Phillip pointed out, makes it
impossible to distinguish between a string which just happens to
contain the two-character sequence '\' and 'n', and an escaped newline
"\n". So, the above should really be:

Alright, I believe I've got the whole picture now and sorry for the
confusion. You and Phillip clearly stated in the review cycle that the
reason should be quoted because of the aforementioned reasons and I
dropped when I was working on this version.

It was my fault by confusing you with the misleading mention of CQUOTE_NODQ.

quoted

quoted

+       git worktree add --detach locked1 &&
+       git worktree add --detach locked2 &&
+       git worktree add --detach unlocked &&

So, the purpose of the `unlocked` worktree in this test is to prove
that it didn't accidentally get annotated with `locked`? (Since, if it
did get annotated, then `actual` would contain too many lines and not
match `expect`.) Is that correct?

Yes, this is what I intended to check when adding the `unlocked`
worktree. I'm considering how to make this more explicit so it's clear
for readers why the `unlocked` worktree exists in this test.

A simple in-code comment should suffice, I would think:

    git worktree add --detach locked1 &&
    git worktree add --detach locked2 &&
    # unlocked worktree should not be annotated "locked"
    git worktree add --detach unlocked &&