Parse --o in format-patch

Parse --o in format-patch

[Fredrik Gustafsson]

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

Re: Parse --o in format-patch

[Jeff King]

On Fri, Jun 28, 2013 at 06:05:00PM +0200, Fredrik Gustafsson wrote:

> 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

Re: Parse --o in format-patch

[Fredrik Gustafsson]

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:
> 
>   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

Re: Parse --o in format-patch

[Jeff King]

On Fri, Jun 28, 2013 at 06:44:40PM +0200, Fredrik Gustafsson wrote:

> 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." :)

> 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.