Re: [PATCH 1/2] git-svn: add some in-code documentation (options-related).

From: Yann Dirson <ydirson@altern.org>
Date: Jun 22, 2009, 19:12 UTC
Message-ID: <20090622191207.GA5590@nan92-1-81-57-214-146.fbx.proxad.net>
In-Reply-To: <7vmy81p2r5.fsf@alter.siamese.dyndns.org>

On Sun, Jun 21, 2009 at 04:48:30PM -0700, Junio C Hamano wrote:

Show 30 quoted lines

> Yann Dirson <ydirson@altern.org> writes:
> 
> > +# Option structures here are used to define both command-line options
> > +# and git-config keys.  They are handled by read-repo-config().
> > +# Options in mixed case and with only a long form are only for the
> > +# config file.  All options available from command-line are available
> > +# as config-file options too, but without any dash in their names.
> 
> Hmm... I am a bit confused.  So for example:
> 
> 		'authors-file|A=s' => \$_authors,
> 		'authors-prog=s' => \$_authors_prog,
> 		'repack:i' => \$Git::SVN::_repack,
> 		'noMetadata' => \$Git::SVN::_no_metadata,
> 		'useSvmProps' => \$Git::SVN::_use_svm_props,
> 
> authors-file, authors-prog and repack are not MixdCase (and the first and
> third are not even "only a long form"), so it is not "only for the config
> file".  But noMetadata and useSvmProps are "only for the config file".
> 
> It took me a bit to realize that these two equivalence hold:
> 
>   "not only for the config file" === "also available as command line option"
>   "only for the config file"     === "cannot be given on the command line"
> 
> Since "option structures here are used for both command and config", and
> "anything usable from command line can be used in config file", logically
> it follows that "everything in these option structures here are usable in
> the config".  It took me a bit longer than necessary for that to click,
> too.

Right, that should probably be emphasized more. I originally did not realize that those also drive config-file settings, right after I discovered how the config items are handled, and this clearly shows :)

Show 11 quoted lines

> After scratching my head for a few minutes, here is a rephrased
> description I came up with (I am not writing this as an improved
> suggestion, but to ask you to sanity check if I read what you wanted to
> say correctly):
> 
>     These are all configurable in the config file (remove dash in their
>     names to use these names as the config key).
> 
>     Options that have MixedCase names without short format (e.g. noMetadata)
>     can only be used from the configuration (iow they cannot be given from
>     the command line as options).

That's what I had got from comments in read_repo_config() and tried to convey in this comment.

> But then this raises a couple of questions.
> 
>  * What are the examples of MixedCase names that do have short format?
>    Are they usable from the command line?

There are no such example, and I suspect this restriction was only mentionned for completeness, as a consequence that config-only items are defined in the code by containing only letters and at least one uppercase (that is, the real condition also encompasses uppercase-only items). I'll try to bring a better formulation.

>  * %fc_opts() has "noMetadata" and %init_opts() has "no-metadata"; how are
>    they related, and how do they affect the above "what's available where
>    and how to spell their names" rule you outlined?

The mixed-case versions are the config items, and the dashed versions are used by "git svn init" to seed these items at init/clone time, so one does not have to use "git svn init", then set config, and only then doing initial fetch.

As a consequence, one can set eg. svn.nometadata in his ~/.gitconfig, and subsequently have all git-svn-cloned repository initialized with svn.noMetadata. That's probably not an example of clarity, and things being error-prone, such an example in the doc would probably be welcomed; I can add this too.

Speaking about the doc, I was wondering whether it would not be better to split my previous patch into 1/3 formatting issues (patch which could probably be applied early), 2/3 shuffling existing options to the right place, 3/3 adding missing options. Afterwards more work can be done for clarification, but that is not my primary goal, I'd rather try to document the internals first.