From: Stephen Boyd <bebarino@gmail.com>
Date: Fri, 10 Jul 2009 08:02:22 GMT
Subject: Re: [PATCH/RFC] grep: Add --directories option.
Message-ID: <4A56F58E.1080907@gmail.com>
In-Reply-To: <1247167228-2466-1-git-send-email-michal.kiedrowicz@gmail.com>

Michał Kiedrowicz wrote:
> @@ -648,10 +658,32 @@ static int help_callback(const struct option *opt, const char *arg, int unset)
>  	return -1;
>  }
>  
> +static int directories_callback(const struct option *opt,
> +				const char *arg, int unset)
> +{
> +	int *recurse = opt->value;
> +
> +	if (!arg)
> +		return error("switch `d' requires a value");

This isn't needed because OPT_CALLBACK requires an argument always be given.

> +
> +	if (!strcmp(arg, "recurse")) {
> +		*recurse = 1;
> +		return 0;
> +	} else if (!strcmp(arg, "skip")) {
> +		*recurse = 0;
> +		return 0;
> +	}
> +
> +	fprintf(stderr, "Invalid action `%s'.\n", arg);
> +	fprintf(stderr, "Available actions are: recurse skip.\n");
> +	return -1;
> +}
> +

I think I would drop the two fprintf's here and just return an error()
saying invalid action. This will in turn cause the usage message to show
up, where you can show the two possible actions.

> @@ -674,6 +706,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
>  		OPT_SET_INT('I', NULL, &opt.binary,
>  			"don't match patterns in binary files",
>  			GREP_BINARY_NOMATCH),
> +		OPT_CALLBACK('d', "directories", &recurse, "action",
> +			"action to perform when input file is a directory",
> +			directories_callback),
>  		OPT_GROUP(""),
>  		OPT_BIT('E', "extended-regexp", &opt.regflags,
>  			"use extended POSIX regular expressions", REG_EXTENDED),

Do you want to allow "--no-directories", I would suggest setting the
PARSE_OPT_NONEG flag to avoid this. Maybe you want to replace "action"
with "recurse|skip" too.