Hi,
Matthew DeVore wrote:
> On Sun, Jun 09, 2019 at 10:17:19AM +0200, Johannes Schindelin wrote:
>> I find that it makes sense in general to suppress one's urges regarding
>> introducing `{ ... }` around one-liners when the patch does not actually
>> require it.
>>
>> For example, I found this patch harder than necessary to read because of
>> it.
>
> I understand the desire to make the patch itself clean, and I sometimes try to
> do that to a fault, but the style as I understand it is to put { } around all
> if branches if only one branch requires it. Since I'm already modifying the
> "else if (cmp_type == FIELD_STR)" line, I decided to put the } at the start of
> the line and modify the if (s->version) line as well. So only one line was
> modified "in excess." I think the temporary cost of the verbose patch is
> justified to keep the style consistent in narrow code fragments.
Git seems to be inconsistent about this. Documentation/CodingGuidelines
says
- When there are multiple arms to a conditional and some of them
require braces, enclose even a single line block in braces for
consistency. E.g.:
so you have some cover from there (and it matches what I'm used to,
too). :)
[...]
>>> + LC_ALL=$zh_CN_locale LC_MESSAGES=$zh_CN_locale \
>>> + git -C r1 branch >actual &&
>>> + git -C r1 checkout - &&
>>
>> Why call `checkout` after `branch`? That's unnecessary, we do not verify
>> anything after that call.
>
> It's to get the repo into a neutral state in case an additional testcase is
> added in the future.
For this kind of thing, we tend to use test_when_finished so that the
test ends up in a clean state even if it fails.
[...]
> test_expect_success GETTEXT_ZH_LOCALE 'detached head sorts before branches' '
> # Ref sorting logic should put detached heads before the other
> # branches, but this is not automatic when a branch name sorts
> # lexically before "(" or the full-width "(" (Unicode codepoint FF08).
> # The latter case is nearly guaranteed for the Chinese locale.
>
> git -C r1 checkout HEAD^{} -- &&
> LC_ALL=$zh_CN_locale LC_MESSAGES=$zh_CN_locale \
> git -C r1 branch >actual &&
> git -C r1 checkout - &&
>
> head -n 1 actual >first &&
> # The first line should be enclosed by full-width parenthesis.
> grep '$'\xef\xbc\x88.*\xef\xbc\x89'' first &&
nit: older shells do not know how to do $'\x01' interpolation.
Probably best to use the raw UTF-8 directly here (it will be more
readable anyway).
Thanks,
Jonathan
