Re: [PATCH v3] parse-opt: migrate builtin-ls-files.
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Feb 14, 2009, 20:56 UTC
- Message-ID
- <alpine.DEB.1.00.0902142127040.10279@pacific.mpi-cbg.de>
- In-Reply-To
- <1231376145-32331-1-git-send-email-vmiklos@frugalware.org>
Hi,
On Thu, 8 Jan 2009, Miklos Vajna wrote:
Show 9 quoted lines
> +static int option_parse_z(const struct option *opt,
> + const char *arg, int unset)
> +{
> + if (unset)
> + line_terminator = '\n';
> + else
> + line_terminator = 0;
> + return 0;
> +}line_terminator = unset ? '\0' : '\n';
Hmm?
Show 7 quoted lines
> +static int option_parse_exclude(const struct option *opt,
> + const char *arg, int unset)
> +{
> + struct dir_struct *dir = opt->value;
> +
> + exc_given = 1;
> + add_exclude(arg, "", 0, &dir->exclude_list[EXC_CMDL]);Why is &dir->exclude_list[EXC_CMDL] not stored in opt->value directly?
Show 9 quoted lines
> +static int option_parse_ignored(const struct option *opt,
> + const char *arg, int unset)
> +{
> + struct dir_struct *dir = opt->value;
> +
> + dir->show_ignored = !unset;
> +
> + return 0;
> +}Maybe this wants to be converted into an OPTION_BIT compatible data type?
Show 9 quoted lines
> +static int option_parse_directory(const struct option *opt,
> + const char *arg, int unset)
> +{
> + struct dir_struct *dir = opt->value;
> +
> + dir->show_other_directories = !unset;
> +
> + return 0;
> +}Likewise?
Show 9 quoted lines
> +static int option_parse_empty(const struct option *opt,
> + const char *arg, int unset)
> +{
> + struct dir_struct *dir = opt->value;
> +
> + dir->hide_empty_directories = unset;
> +
> + return 0;
> +}Maybe we need an OPT_BIT_NEG?
Show 7 quoted lines
> +static int option_parse_full_name(const struct option *opt,
> + const char *arg, int unset)
> +{
> + prefix_offset = 0;
> +
> + return 0;
> +}Not that it matters much, but maybe you can guard this with an "if (!unset)"?
> + { OPTION_CALLBACK, 0, "full-name", NULL, NULL,
> + "make the output relative to the project top directory",
> + PARSE_OPT_NOARG, option_parse_full_name },Maybe OPT_NONEG, and maybe SET_INT?
> + if (dir.exclude_per_dir) > + exc_given = 1;
You could use a boolean to handle --exclude-standard, too... But you did not do that so that there is no regression with specific ordering of the exclude options, right?
Ciao, Dscho