Re: [PATCH v2 1/2] rev-parse: fix segfault with missing --path-format argument

["Wolfgang Müller" <wolf@oriole.systems>]

On 2021-05-17 04:16, Jeff King wrote:

> I don't have a strong preference for one or the other. It is maybe
> helpful to diagnose "--path-format" without an equals as an error, as
> your patch would, rather than quietly passing it along as an unknown
> (as the hunk above would). I dunno. That perhaps applies to the rest
> of the options, too. :)

I have a very slight preference for throwing an error: that is what I
expected to happen as a user. At the same time, passing it along as an
unknown seems more consistent with the other options that take equals.
I'm split on this, and would defer to what people here prefer.

Short of fully migrating to the parse-options API, I do not see a
perfect solution for this, especially since there are options that take
optional arguments which are not delimited by equals. These seem to be
sprinkled throughout and all error out if no argument is given.

Here's a small selection:

	Option                   Section in manual        Parser

	--default <arg>          Options for Output       manual
	--prefix <arg>           Options for Output       manual
	--short[=length]         Options for Output       opt_with_value
	--path-format=[..]       Options for Files        opt_with_value
	--git-path <path>        Options for Files        manual
	--disambiguate=<prefix>  Options for Objects      skip_prefix

Out of curiosity I decided to dig around a bit, my hunch being that
arguments without equals are older.

I can trace --default back to 178cb24338 (Add 'git-rev-parse' helper
script, 2005-06-13). That seems to be the very first version of
git-rev-parse.

The first options with equals, --since= et al., were added in c1babb1d65
([PATCH] Teach "git-rev-parse" about date-based cut-offs, 2005-09-20)

The --short option used to be --abbrev, and was added in d50125085a
(rev-parse: --abbrev option., 2006-01-25). Then, quite a bit later, the
second option without equals was added in abc06822af (rev-parse: add
option --resolve-git-dir <path>, 2011-08-15).

--prefix goes back to 12b9d32790 (rev-parse: add --prefix option,
2013-06-16) and --git-path is 557bd833bb (git_path(): be aware of file
relocation in $GIT_DIR, 2014-11-30)

So it turns out that my hunch was not really correct. Maybe there's also
a pattern that I am not seeing. Obviously this has no bearing on the
segfault fix, but is maybe valuable information for a conversion to the
parse-options API down the line. It was also fun to figure out, since I
could not stop thinking about rev-parse having a bunch of different
option semantics.

-- 
Wolfgang