Re: [PATCH v6 04/11] test-regex: expose full regcomp() to the command line

[Junio C Hamano]

Eric Sunshine  writes:

> On Fri, Feb 5, 2016 at 9:03 PM, Nguyễn Thái Ngọc Duy  
> wrote:
>> diff --git a/test-regex.c b/test-regex.c
>> @@ -21,8 +38,38 @@ static int test_regex_bug(void)
>>  int main(int argc, char **argv)
>>  {
>> +   const char *pat;
>> +   const char *str;
>> +   int flags = 0;
>> +   regex_t r;
>> +   regmatch_t m[1];
>> +
>> if (argc == 2 && !strcmp(argv[1], "--bug"))
>> return test_regex_bug();
>> -   else
>> -   die("must be used with --bug");
>> +   else if (argc < 3)
>> +   die("usage: test-regex --bug\n"
>> +   "   test-regex   []");
>
> This is just a test program, so it probably isn't that important, but
> die() automatically prepends "fatal: " which means the alignment of
> the second line will be wrong. Perhaps you want to use usage() instead
> which automatically prepends "usage: " (and drop the literal "usage: "
> from the above string).

So true.

>> +   argv++;
>> +   pat = *argv++;
>> +   str = *argv++;
>> +   while (*argv) {
>> +   struct reg_flag *rf;
>> +   for (rf = reg_flags; rf->name; rf++)
>> +   if (!strcmp(*argv, rf->name)) {
>> +   flags |= rf->flag;
>> +   break;
>> +   }
>> +   if (!rf->name)
>> +   die("do not recognize %s", *argv);
>> +   argv++;
>> +   }
>> +   git_setup_gettext();
>> +
>> +   if (regcomp(, pat, flags))
>> +   die("failed regcomp() for pattern '%s'", pat);
>> +   if (regexec(, str, 1, m, 0))
>> +   return 1;
>> +
>> +   return 0;
>>  }
>
> This version is much easier to read without the "bug" special case
> spread throughout the code. Thanks.
--
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

Re: [PATCH v6 04/11] test-regex: expose full regcomp() to the command line

[Eric Sunshine]

On Fri, Feb 5, 2016 at 9:03 PM, Nguyễn Thái Ngọc Duy  wrote:
> diff --git a/test-regex.c b/test-regex.c
> @@ -21,8 +38,38 @@ static int test_regex_bug(void)
>  int main(int argc, char **argv)
>  {
> +   const char *pat;
> +   const char *str;
> +   int flags = 0;
> +   regex_t r;
> +   regmatch_t m[1];
> +
> if (argc == 2 && !strcmp(argv[1], "--bug"))
> return test_regex_bug();
> -   else
> -   die("must be used with --bug");
> +   else if (argc < 3)
> +   die("usage: test-regex --bug\n"
> +   "   test-regex   []");

This is just a test program, so it probably isn't that important, but
die() automatically prepends "fatal: " which means the alignment of
the second line will be wrong. Perhaps you want to use usage() instead
which automatically prepends "usage: " (and drop the literal "usage: "
from the above string).

> +   argv++;
> +   pat = *argv++;
> +   str = *argv++;
> +   while (*argv) {
> +   struct reg_flag *rf;
> +   for (rf = reg_flags; rf->name; rf++)
> +   if (!strcmp(*argv, rf->name)) {
> +   flags |= rf->flag;
> +   break;
> +   }
> +   if (!rf->name)
> +   die("do not recognize %s", *argv);
> +   argv++;
> +   }
> +   git_setup_gettext();
> +
> +   if (regcomp(, pat, flags))
> +   die("failed regcomp() for pattern '%s'", pat);
> +   if (regexec(, str, 1, m, 0))
> +   return 1;
> +
> +   return 0;
>  }

This version is much easier to read without the "bug" special case
spread throughout the code. Thanks.
--
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