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 <larsxschneider@xxxxxxxxx> > Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx> > Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> > > --- [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