On Fri, May 6, 2016 at 10:49 AM, Pranit Bauva <[email protected]> wrote:
> Reimplement the `check_term_format` shell function in C and add
> a `--check-term-format` subcommand to `git bisect--helper` to call it
> from git-bisect.sh
>
> Using `--check-term-format` subcommand is a temporary measure to port
> shell function to C so as to use the existing test suite. As more
> functions are ported, this subcommand will loose its existence and will
s/loose/lose/
Though, maybe it would be better to say:
...this subcommand will be retired...
> be called by some other method/subcommand. For eg. In conversion of
> write_terms() of git-bisect.sh, the subcommand will be removed and
> instead check_term_format() will be called in its C implementation while
> a new subcommand will be introduced for write_terms().
>
> Signed-off-by: Pranit Bauva <[email protected]>
> ---
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> @@ -2,16 +2,65 @@
> +/*
> + * To check whether the string `term` belongs to the set of strings
> + * included in the variable arguments.
This is still a sentence fragment as pointed out last time[1]. You can
fix it with: s/To check/Check/
> + */
> +static int one_of(const char *term, ...)
> +{
> + int res = 0;
> + va_list matches;
> + const char *match;
> +
> + va_start(matches, term);
> + while (!res && (match=va_arg(matches, const char *)))
Style: add spaces around '='
> + res = !strcmp(term, match);
> + va_end(matches);
> +
> + return res;
> +}
The rest of the patch seems to address the issues raised by my
previous review[1] (aside from my comments[1,2] about this bottom-up
approach repeatedly adding and removing unnecessary complexity as each
little function is ported from shell to C, but that's a separate
philosophical discussion).
[1]: http://thread.gmane.org/gmane.comp.version-control.git/289476/focus=293501
[2]: http://thread.gmane.org/gmane.comp.version-control.git/289476/focus=293517
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
