Ævar Arnfjörð Bjarmason <[email protected]> writes:
> diff --git a/t/t0014-abbrev.sh b/t/t0014-abbrev.sh
> new file mode 100755
> index 0000000000..1c60f5ff93
> --- /dev/null
> +++ b/t/t0014-abbrev.sh
> @@ -0,0 +1,118 @@
> +#!/bin/sh
> +
> +test_description='test core.abbrev and related features'
> +
> +. ./test-lib.sh
> +
> +tr_d_n() {
> + tr -d '\n'
> +}
- I personally would prefer to see the reason for having a helper
function like this to be "make it easier to reason about", rather
"make it shorter to type". tr_d_n feels more about the latter; if
we aimed for the former, this would be called strip_LF or
something.
- In the existing stcipts, it seems that we prefer to spell tr args
with 3-octal, e.g. \012 for LF, \015 for CR.
- Also let's have SP on both sides of (), i.e.
funcname () {
for shell function definitions.
> +cut_tr_d_n_field_n() {
> + cut -d " " -f $1 | tr_d_n
> +}
Likewise. Name this not after how it does what it does, but after
what it does and why it does so. In other words, if your answer to
the question: "what does the caller want?" is "it wants to pick the
nth field", then name it "pick_nth_field" or something?
> +test_expect_success 'abbrev empty value handling differs ' '
> + test_must_fail git -c core.abbrev= log -1 --pretty=format:%h 2>stderr &&
> + test_i18ngrep "bad numeric config value.*invalid unit" stderr &&
> +
> + git branch -v --abbrev= | cut_tr_d_n_field_n 3 >branch &&
> + test_byte_count = 40 branch &&
Sounds like a good thing to unify. If anything, --options=value
should be stricter than vari.able=value but it is the other way
around.
> + git log --abbrev= -1 --pretty=format:%h >log &&
> + test_byte_count = 4 log &&
Makes readers wonder if 4 is about 3 hex plus terminating LF. The
reason why this works is because --pretty=format:%h (not --format=%h
or --pretty=tformat:%h) uses delimiter semantics and we won't need
any LF to show a single record.
If we use the helper to measure the length of hexadecimal digits, it
may make sense to add a wrapper around test_byte_count that strips
LF; that way, the caller can use --format=%h instead and there will
be one less thing for the reader to worry about.
> + git diff --raw --abbrev= HEAD~ >diff &&
> + cut_tr_d_n_field_n 3 <diff >diff.3 &&
> + test_byte_count = 4 diff.3 &&
> + cut_tr_d_n_field_n 4 <diff >diff.4 &&
> + test_byte_count = 4 diff.4 &&
These all depend on the fact that we do not have excessive number of
irrelevant objects to force us to abbreviate using more hexdigits
than the minimum 4, right? I _think_ that is a reasonable
assumption we can depend on, even across the hash function
transition. We may want to leave in-code comment, though.
> + test_must_fail git diff --raw --abbrev= --no-index X Y >diff &&
> + cut_tr_d_n_field_n 3 <diff >diff.3 &&
> + test_byte_count = 4 diff.3 &&
