Re: [PATCH v12 13/13] bisect--helper: `bisect_start` shell function partially in C
- From
Christian Couder <christian.couder@gmail.com>
- Date
- Aug 13, 2016, 07:34 UTC
- Message-ID
- <CAP8UFD1pUOUKKMQLsyTMtSBUG3s340vOjrGqRpC-CGzuyQXwzA@mail.gmail.com>
- In-Reply-To
- <010201567675ae72-0e0eeb11-e488-4562-8837-2443cbfe0792-000000@eu-west-1.amazonses.com>
On Wed, Aug 10, 2016 at 11:57 PM, Pranit Bauva <pranit.bauva@gmail.com> wrote:
Show 29 quoted lines
>
> @@ -431,6 +434,244 @@ static int bisect_terms(struct bisect_terms *terms, const char **argv, int argc)
> return 0;
> }
>
> +static int bisect_start(struct bisect_terms *terms, int no_checkout,
> + const char **argv, int argc)
> +{
> + int i, has_double_dash = 0, must_write_terms = 0, bad_seen = 0;
> + int flags, pathspec_pos;
> + struct string_list revs = STRING_LIST_INIT_DUP;
> + struct string_list states = STRING_LIST_INIT_DUP;
> + struct strbuf start_head = STRBUF_INIT;
> + struct strbuf bisect_names = STRBUF_INIT;
> + struct strbuf orig_args = STRBUF_INIT;
> + const char *head;
> + unsigned char sha1[20];
> + FILE *fp;
> + struct object_id oid;
> +
> + if (is_bare_repository())
> + no_checkout = 1;
> +
> + for (i = 0; i < argc; i++) {
> + char *commit_id = xstrfmt("%s^{commit}", argv[i]);
> + if (!strcmp(argv[i], "--")) {
> + has_double_dash = 1;
> + break;
> + }In the shell code there is a loop dedicated to checking if there is a double dash in the arguments before the real argument parsing loop. There is a reason for that. If you do it in the same loop, has_double_dash will not be set when the arguments that are before the double dash will be parsed.
Show 9 quoted lines
> + else if (!strcmp(argv[i], "--no-checkout"))
> + no_checkout = 1;
> + else if (!strcmp(argv[i], "--term-good") ||
> + !strcmp(argv[i], "--term-old")) {
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_good);
> + strbuf_addstr(&terms->term_good, argv[++i]);
> + }
> + else if (skip_prefix(argv[i], "--term-good=", &argv[i])) {(Maybe you could put the "else if (...) {" on the same line as the "}" above.)Show 32 quoted lines
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_good);
> + strbuf_addstr(&terms->term_good, argv[i]);
> + }
> + else if (skip_prefix(argv[i], "--term-old=", &argv[i])) {
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_good);
> + strbuf_addstr(&terms->term_good, argv[i]);
> + }
> + else if (!strcmp(argv[i], "--term-bad") ||
> + !strcmp(argv[i], "--term-new")) {
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_bad);
> + strbuf_addstr(&terms->term_bad, argv[++i]);
> + }
> + else if (skip_prefix(argv[i], "--term-bad=", &argv[i])) {
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_bad);
> + strbuf_addstr(&terms->term_bad, argv[i]);
> + }
> + else if (skip_prefix(argv[i], "--term-new=", &argv[i])) {
> + must_write_terms = 1;
> + strbuf_reset(&terms->term_good);
> + strbuf_addstr(&terms->term_good, argv[i]);
> + }
> + else if (starts_with(argv[i], "--") &&
> + !one_of(argv[i], "--term-good", "--term-bad", NULL)) {
> + string_list_clear(&revs, 0);
> + string_list_clear(&states, 0);
> + die(_("unrecognised option: '%s'"), argv[i]);
> + }
> + else if (get_oid(argv[i], &oid) && !has_double_dash) {And here checking "!has_double_dash" has no meaning if you check for a double dash in the same loop, because there is a "break" after has_double_dash is set above.
Show 12 quoted lines
> + string_list_clear(&revs, 0);
> + string_list_clear(&states, 0);
> + free(commit_id);
> + die(_("'%s' does not appear to be a valid revision"), argv[i]);
> + }
> + else {
> + free(commit_id);
> + string_list_append(&revs, oid_to_hex(&oid));
> + }
> + }
> + pathspec_pos = i;
> +