threads / discuss / 34301

Parse --o in format-patch

Subject: Parse --o in format-patch

## tl;dr

4 messages between Jun 28, 2013 and Jun 28, 2013.

replies: 3people: 2as markdown or json

Fredrik Gustafsson· Jun 28, 2013, 16:05 UTC · lore

Hi, I don't quite manage to figure out gits argv parsing and would need some help on the way.

I want: git format-patch -o outdir HEAD~

Work exactly the way it does now, setting output_directory to outdir. But I also want git format-patch -o HEAD~

to set output_directory with a NULL value so that I can assign a default value to it. Is that even possible with the current argv-parsing implementation?

The currect argv parser is using OPTION_CALLBACK so I thought that that callback should be able to determine if there was an outdir supplied or not.

Or is the correct solution to also add a bolean type OPTION_BOOLEAN for -o?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com
Jeff King· Jun 28, 2013, 16:31 UTC · re: Fredrik Gustafsson · lore

Re: Parse --o in format-patch

On Fri, Jun 28, 2013 at 06:05:00PM +0200, Fredrik Gustafsson wrote:
Show 12 quoted lines
> I don't quite manage to figure out gits argv parsing and would need some
> help on the way.
> 
> I want:
> git format-patch -o outdir HEAD~
> 
> Work exactly the way it does now, setting output_directory to outdir.
> But I also want
> git format-patch -o HEAD~
> 
> to set output_directory with a NULL value so that I can assign a default
> value to it. Is that even possible with the current argv-parsing implementation?

It's possible to have an "optional" argument by using the PARSE_OPT_OPTARG flag. However, it is not backwards compatible from the user's perspective, as they must use the "sticked" form:

  git format-patch -ooutdir ...
to specify the argument. Otherwise, it is not clear in:
  git format-patch -o outdir HEAD~

whether "outdir" is the argument to "-o", or if it is simply the next argument.

However, if you are just interested in "turning off" a previously given argument, we usually spell that with the "--no-" prefix to the long option (e.g., "--no-output-directory" in this case).

I'm not clear how that interacts with your example, though. Your example tries to use "-o" to set output_directory to NULL. But it is already NULL if you do not specify any "-o" at all.

-Peff
Fredrik Gustafsson· Jun 28, 2013, 16:44 UTC · re: Jeff King · lore

Re: Parse --o in format-patch

On Fri, Jun 28, 2013 at 12:31:53PM -0400, Jeff King wrote:
Show 12 quoted lines
> It's possible to have an "optional" argument by using the
> PARSE_OPT_OPTARG flag. However, it is not backwards compatible from the
> user's perspective, as they must use the "sticked" form:
> 
>   git format-patch -ooutdir ...
> 
> to specify the argument. Otherwise, it is not clear in:
> 
>   git format-patch -o outdir HEAD~
> 
> whether "outdir" is the argument to "-o", or if it is simply the next
> argument.

That would be a possibility but I don't like breaking backwards compability.

> I'm not clear how that interacts with your example, though. Your example
> tries to use "-o" to set output_directory to NULL. But it is already
> NULL if you do not specify any "-o" at all.
my goal is to make:
       git format-patch [-k] [(-o|--output-directory) <dir> | --stdout] [ <since> | <revision range> ]
to be:
       git format-patch [-k] [(-o|--output-directory) [dir] | --stdout] [ <since> | <revision range> ]

that would do: git format patch -> current dir git format patch -o -> default dir (for example GIT_DIR/.outgoing/) git format patch -o <dir> -> user defined <dir>

But I guess I would need a new option instead. Something like --default-output-dir.

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com
Jeff King· Jun 28, 2013, 16:49 UTC · re: Fredrik Gustafsson · lore

Re: Parse --o in format-patch

On Fri, Jun 28, 2013 at 06:44:40PM +0200, Fredrik Gustafsson wrote:
Show 7 quoted lines
> On Fri, Jun 28, 2013 at 12:31:53PM -0400, Jeff King wrote:
> > It's possible to have an "optional" argument by using the
> > PARSE_OPT_OPTARG flag. However, it is not backwards compatible from the
> > user's perspective, as they must use the "sticked" form:
> 
> That would be a possibility but I don't like breaking backwards
> compability.

Yes, I did not say it outright, but I meant "...and that is why we cannot go that route." :)

Show 9 quoted lines
> my goal is to make:
>        git format-patch [-k] [(-o|--output-directory) <dir> | --stdout] [ <since> | <revision range> ]
> to be:
>        git format-patch [-k] [(-o|--output-directory) [dir] | --stdout] [ <since> | <revision range> ]
> 
> that would do:
> git format patch -> current dir
> git format patch -o -> default dir (for example GIT_DIR/.outgoing/)
> git format patch -o <dir> -> user defined <dir>
Ah, that makes much more sense to me.
> But I guess I would need a new option instead. Something like
> --default-output-dir.

It depends on how the default is specified. Is it hard-coded? Or do you specify format.outputDirectory? If the latter, I would think you would want it on all the time when "-o" is not given[1], and no new option is required. Otherwise, yes, I'd think you would want a new option.

-Peff
[1] format-patch may be considered plumbing, in which case an output
    directory config option might cause problems with scripts that
    expect to run it and find the output in the current directory. I'm
    not sure how big a deal that is.

← back to recent threads