On Mon, Feb 23, 2015 at 01:07:13PM -0800, Junio C Hamano wrote:
> >> - OPT_STRING(0, "template", &option_template, N_("template-directory"),
> >> + OPT_PATH(0, "template", &option_template, N_("template-directory"),
> >> N_("directory from which templates will be used")),
> >> OPT_CALLBACK(0 , "reference", &option_reference, N_("repo"),
> >> N_("reference repository"), &opt_parse_reference),
> >
> > I'm not sure if this one is doing anything. Clone cannot use SETUP_GIT
> > for obvious reasons, so we should have a NULL prefix here. But that also
> > means we should be doing the right thing already.
>
> I somehow thought that OPT_FILENAME already used expand_user_path()
> but apparently it does not. It may want to.
>
> And then this change will start to matter, as a good enhancement.
>
> Of course, if OPT_PATH() is introduced in such a way that the
> program that uses the API can ask for "existing" and/or "directory",
>
> git clone --template=existing-file $URL
> git clone --template=no-such-directory $URL
>
> can be diagnosed as an error without the program having to code very
> much.
>
> So, I agree that this change does not do anything in the current
> codebase, but it goes in a right direction.
Oh, I agree that annotating the option with more meaning is not a bad
thing. It may help readability, and as you note, we may grow new
features on those annotations over time. I mostly just got tired of
writing things along those lines by the time I finished with the
OPT_FILENAME patch (I guess OPT_PATH is the one more likely to grow new
features, though).
The one exception here is the thing I mentioned in parse_archive_args:
that one can be controlled by a remote caller and we would not want to
leak any information about which files exist, where the user's home
directory is, etc.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
