Adam Spiers <git@xxxxxxxxxxxxxx> writes:
> diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c
> new file mode 100644
> index 0000000..c825736
> --- /dev/null
> +++ b/builtin/check-ignore.c
> @@ -0,0 +1,170 @@
> +#include "builtin.h"
> +#include "cache.h"
> +#include "dir.h"
> +#include "quote.h"
> +#include "pathspec.h"
> +#include "parse-options.h"
> +
> +static int quiet, verbose, stdin_paths;
> +static const char * const check_ignore_usage[] = {
> +"git check-ignore [options] pathname...",
> +"git check-ignore [options] --stdin < <list-of-paths>",
> +NULL
> +};
> +
> +static int null_term_line;
> +
> +static const struct option check_ignore_options[] = {
> + OPT__QUIET(&quiet, N_("suppress progress reporting")),
> + OPT__VERBOSE(&verbose, N_("be verbose")),
> + OPT_GROUP(""),
> + OPT_BOOLEAN(0, "stdin", &stdin_paths,
> + N_("read file names from stdin")),
> + OPT_BOOLEAN('z', NULL, &null_term_line,
> + N_("input paths are terminated by a null character")),
> + OPT_END()
> +};
> +
> +static void output_exclude(const char *path, struct exclude *exclude)
> +{
> + char *bang = exclude->flags & EXC_FLAG_NEGATIVE ? "!" : "";
> + char *dir = (exclude->flags & EXC_FLAG_MUSTBEDIR) ? "/" : "";
That's inconsistent. Either s/bang/negated/ or s/dir/slash/ (but
obviously not both).
> +static int check_ignore(const char *prefix, const char **pathspec)
> +{
> + struct dir_struct dir;
> +...
> + if (pathspec) {
> +...
> + } else {
> + printf("no pathspec\n");
> + }
Is this an error message, an informative warning, or what? The
command is presumably for script consumption downstream of a pipe.
Does it make sense to emit this message to their standard input?
Does it even have to be output? Especially what should happen when
the user says "--quiet"?
Perhaps
if (!quiet)
fprintf(stderr, "no pathspec given.\n");
or something?
> +int cmd_check_ignore(int argc, const char **argv, const char *prefix)
> +{
> + int num_ignored = 0;
I do not think you need to initialize this one.
> + if (stdin_paths) {
> + num_ignored = check_ignore_stdin_paths(prefix);
> + } else {
> + num_ignored = check_ignore(prefix, argv);
> + maybe_flush_or_die(stdout, "ignore to stdout");
> + }
> +
> + return num_ignored > 0 ? 0 : 1;
Given that num_ignored will always be >=0, that is a funny way to
say
return !num_ignored;
or if you plan to report a non-fatal error as negative return from
the two check_ignore* functions, perhaps:
return num_ignored <= 0;
> diff --git a/t/t0008-ignores.sh b/t/t0008-ignores.sh
> new file mode 100755
> index 0000000..3937ca4
> --- /dev/null
> +++ b/t/t0008-ignores.sh
> @@ -0,0 +1,595 @@
> +#!/bin/sh
> +
> +test_description=check-ignore
> +
> +. ./test-lib.sh
> +
> +init_vars () {
> + global_excludes="$(pwd)/global-excludes"
> +}
> +
> +enable_global_excludes () {
> + init_vars
> + git config core.excludesfile "$global_excludes"
> +}
> +
> +expect_in () {
> + dest="$HOME/expected-$1" text="$2"
> + if test -z "$text"
> + then
> + >"$dest" # avoid newline
> + else
> + echo "$text" >"$dest"
> + fi
> +}
> +
> +expect () {
> + expect_in stdout "$1"
> +}
> +
> +expect_from_stdin () {
> + cat >"$HOME/expected-stdout"
> +}
> +
> +test_stderr () {
> + expected="$1"
> + expect_in stderr "$1" &&
> + test_cmp "$HOME/expected-stderr" "$HOME/stderr"
> +}
> +
> +stderr_contains () {
> + regexp="$1"
> + if grep -q "$regexp" "$HOME/stderr"
Please do not use "grep -q"; the output from commands in test
harness is normally hidden already. This only makes script more
cluttered and robs debuggers of potentially useful output when the
test script is run with "-v".
> +test_check_ignore () {
> + args="$1" expect_code="${2:-0}" global_args="$3"
> +
> + init_vars &&
> + rm -f "$HOME/stdout" "$HOME/stderr" "$HOME/cmd" &&
> + echo git $global_args check-ignore $quiet_opt $verbose_opt $args \
> + >"$HOME/cmd" &&
Does a debugger needs this "cmd" file when we already have "-v" option?
> +test_expect_success_multi () {
> + prereq=
> + if test $# -eq 4
> + then
> + prereq=$1
> + shift
> + fi
> + testname="$1" expect_verbose="$2" code="$3"
> +
> + expect=$( echo "$expect_verbose" | sed -e 's/.* //' )
> +
> + test_expect_success $prereq "$testname" "
> + expect '$expect' &&
> + $code
> + "
This is brittle when $expect may have single quotes, isn't it?
Perhaps
test_expect_success $prereq "$testname" '
expect "$expect" && eval "$code"
'
may fix it, but in general I hate to see each test script trying to
invent their own mini-harness like this (and getting them wrong).
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html