"Derrick Stolee via GitGitGadget" <[email protected]> writes:
> +files=$(git diff --name-only $V1 $V2 -- *.c)
You'd want to quote that *.c from the shell, i.e. either one of
these
files=$(git diff --name-only $V1 $V2 -- \*.c)
files=$(git diff --name-only $V1 $V2 -- '*.c')
otherwise you'd lose things like "builtin/am.c", I'd think.
> +
> +for file in $files
> +do
I know this is only for running in _our_ source tree, and we do not
have a source with $IFS in it, so I'd declare that this is OK. It
would be good to document that assumption in red capital letters at
the beginning of this loop, though ;-)
# Note: this script is only for our codebase and we rely on
# the fact that the pathnames of our source files do not
# have any funny characters---letting the shell split $files
# list at $IFS boundary is very much intentional, and not
# quoting "$file" in the code below also is. Don't imitate
# this in scripts that are meant to handle random end-user
# repositories!
for file in $files
do
...
> + git diff $V1 $V2 -- $file \
> + | diff_lines \
> + | sort >new_lines.txt
I do not see a strong reason why we would want to limit $V1 and $V2
to branch names and raw commit object names, and quoting them in dq
pair is a cheap fix to allow things like
$ contrib/coverage-diff.sh master 'pu^{/^### match next}'
so let's do so.
Could you cut lines _after_ typing a pipe and omit backslashes, i.e.
git diff "$V1" "$V2" -- $file |
diff_lines |
sort >new_lines.txt
It seems to be personal taste whether to indent the second and
subsequent lines; I do not care if you indent or if you align too
much either way (but I have moderate perference to align).
But I do not want to see people type unnecessary backslashes. This
is not limited to just this pipeline but elsewhere in the script.
> + if ! test -s new_lines.txt
> + then
> + continue
> + fi
> +
> + hash_file=$(echo $file | sed "s/\//\#/")
> + sed -ne '/#####:/{
> + s/ #####://
> + s/:.*//
> + s/ //g
> + p
> + }' "$hash_file.gcov" \
> + | sort >uncovered_lines.txt
> +
> + comm -12 uncovered_lines.txt new_lines.txt \
> + | sed -e 's/$/\)/' \
> + | sed -e 's/^/\t/' \
Do you need two sed invocations for this, or would
sed -e 's/$/\)/' -e '/^/ /'
work as well? By the way """The meaning of an unescaped <backslash>
immediately followed by any character other than '&', <backslash>, a
digit, <newline>, or the delimiter character used for this command,
is unspecified."""[*1*] so '\t' on the replacement side is a no-no
in the portability department.
Reference. *1*
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html
> + >uncovered_new_lines.txt
> +
> + grep -q '[^[:space:]]' < uncovered_new_lines.txt && \
Lose the backslash at the end. The shell knows that you haven't
finished your sentence when it sees a line that ends with &&, || or
a pipe |, so there is no need to tell it redundantly that you have
more things to say with the backslash.
> + echo $file && \
> + git blame -c $file \
> + | grep -f uncovered_new_lines.txt
> +
> + rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt
> +done
Near the begininng (like just before the "for file in $files" loop),
you can probably have a trap to make sure these are removed upon
exit, e.g.
trap 'rm -f new_lines.txt uncovered_lines.txt uncovered_new_lines.txt' 0
