On 27/10/17 16:06, Pranit Bauva wrote:
> Reimplement `bisect_reset` shell function in C and add a `--bisect-reset`
> subcommand to `git bisect--helper` to call it from git-bisect.sh .
>
> Using `bisect_reset` subcommand is a temporary measure to port shell
> functions to C so as to use the existing test suite. As more functions
> are ported, this subcommand would be retired but its implementation will
> be called by some other method.
>
> Note: --bisect-clean-state subcommand has not been retired as there are
> still a function namely `bisect_start()` which still uses this
> subcommand.
>
> Mentored-by: Lars Schneider <[email protected]>
> Mentored-by: Christian Couder <[email protected]>
> Signed-off-by: Pranit Bauva <[email protected]>
>
> ---
[snip]
Sorry for not responding sooner, I've been a bit busy.
Unfortunately, I have only had time to skim the patches, but
I haven't noticed anything too serious.
> builtin/bisect--helper.c | 49
> +++++++++++++++++++++++++++++++++++++++++++++++-
> git-bisect.sh | 28 ++-------------------------
> 2 files changed, 50 insertions(+), 27 deletions(-)
>
> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c
> index 35d2105f941c6..12754448f7b6a 100644
> --- a/builtin/bisect--helper.c
> +++ b/builtin/bisect--helper.c
> @@ -3,15 +3,21 @@
> #include "parse-options.h"
> #include "bisect.h"
> #include "refs.h"
> +#include "dir.h"
> +#include "argv-array.h"
> +#include "run-command.h"
>
> static GIT_PATH_FUNC(git_path_bisect_terms, "BISECT_TERMS")
> static GIT_PATH_FUNC(git_path_bisect_expected_rev, "BISECT_EXPECTED_REV")
> static GIT_PATH_FUNC(git_path_bisect_ancestors_ok, "BISECT_ANCESTORS_OK")
> +static GIT_PATH_FUNC(git_path_bisect_start, "BISECT_START")
> +static GIT_PATH_FUNC(git_path_bisect_head, "BISECT_HEAD")
>
> static const char * const git_bisect_helper_usage[] = {
> N_("git bisect--helper --next-all [--no-checkout]"),
> N_("git bisect--helper --write-terms <bad_term> <good_term>"),
> N_("git bisect--helper --bisect-clean-state"),
> + N_("git bisect--helper --bisect-reset [<commit>]"),
> NULL
> };
>
> @@ -106,13 +112,48 @@ static void check_expected_revs(const char **revs, int
> rev_nr)
> }
> }
>
> +static int bisect_reset(const char *commit)
> +{
> + struct strbuf branch = STRBUF_INIT;
> +
> + if (!commit) {
> + if (strbuf_read_file(&branch, git_path_bisect_start(), 0) < 1)
> + return !printf(_("We are not bisecting.\n"));
I've no idea what this is about! If printf encounters an error, then
this will be equivalent to !-1. If printf does not encounter an error,
then this will be !<length of output> (whatever that may be, given that
the string is marked for translation).
I would suggest that you don't want to do that. ;-)
ATB,
Ramsay Jones
