Hey Christian,
On Sat, Aug 13, 2016 at 1:04 PM, Christian Couder
<christian.cou...@gmail.com> wrote:
> On Wed, Aug 10, 2016 at 11:57 PM, Pranit Bauva <pranit.ba...@gmail.com> wrote:
>>
>> @@ -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.
I had tried that before. But somehow I couldn't get it to run so I
reverted back. I have now successfully tried it and its working. You
can checkout the PR[1].
>> + 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.)
>
>> + 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.
I think this is the situation in the shell script too.
>> + 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;
>> +
Regards,
Pranit Bauva
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
