Re: [PATCH v16 Part II 1/8] bisect--helper: `bisect_reset` shell function in C
- From
Ramsay Jones <ramsay@ramsayjones.plus.com>
- Date
- Nov 8, 2017, 00:07 UTC
- Message-ID
- <272aaa88-99c2-83a8-7343-c65797566a14@ramsayjones.plus.com>
- In-Reply-To
- <0102015f5e5ee171-f30f4868-886f-47a1-a4e4-b4936afc545d-000000@eu-west-1.amazonses.com>
On 27/10/17 16:06, Pranit Bauva wrote:
Show 17 quoted lines
> 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@gmail.com> > Mentored-by: Christian Couder <chriscool@tuxfamily.org> > Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> > > ---
[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.
Show 41 quoted lines
> 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