From: Michael J Gruber <git@drmicha•warpmail.net>
To: git@vger•kernel.org
Cc: git@vger•kernel.org, "Jeff King" <peff@peff•net>,
"Nguye^~n Thái Ngo.c Duy" <pclouds@gmail•com>
Subject: Re: [PATCH 1/4] parse-options: introduce OPT_PATH
Date: Tue, 24 Feb 2015 16:49:25 +0100 [thread overview]
Message-ID: <54EC9D85.3010301@drmicha.warpmail.net> (raw)
In-Reply-To: <xmqqbnkktoti.fsf@gitster.dls.corp.google.com>
Junio C Hamano venit, vidit, dixit 23.02.2015 20:23:
> Michael J Gruber <git@drmicha•warpmail.net> writes:
>
>> Many options are paths, but not files. Introduce OPT_PATH which does
>> the same path processing as OPT_FILENAME but allows to name the argument.
>> ...
>> diff --git a/parse-options.h b/parse-options.h
>> index 7940bc7..5127a5d 100644
>> --- a/parse-options.h
>> +++ b/parse-options.h
>> @@ -149,6 +149,8 @@ struct option {
>> PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }
>> #define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \
>> N_("file"), (h) }
>> +#define OPT_PATH(s, l, v, a, h) { OPTION_FILENAME, (s), (l), (v), \
>> + (a), (h) }
>
> I am somewhat disappointed to see this implementation.
>
> - I expected that OPT_FILENAME will be re-implemented in terms of
> OPT_PATH(), as a thin wrapper.
Right now they are just two macros. Would
#define OPT_FILENAME(s, l, v, h) OPT_PATH((s), (l), (v), N_("file"), (h))
be more what you expect? I don't consider that thinner but don't mind
either way.
> - As the original complaint was "checkout --to requires a
> directory, and a file would not work", I expected this to give
> the programmer finer controls, such as:
>
> - The name must refer to an existing entity on the filesystem,
> or an existing entity on the filesystem must not exist, or
> anything is fine (tristate).
>
> - The name refers to a diretory, or the name refers to a regular
> file, or the name refers to a symbolic link, or anything goes.
>
> That is merely "somewhat", as the latter _can_ be coded (e.g. if you
> care that the given path already exists as a directory, stat() it
> and see if it is, or if you want that the given path does not exist
> yet, stat() it to make sure you get ENOENT) after the option is
> parsed by the program that uses the parser.
>
> But the infrastructure to allow the latter would free you from
> having to say N_("file") or N_("directory"); if a parameter can
> refer to either a file or a directory, the parse-options library
> could give you N_("file or directory") because you are already
> telling what kind(s) of paths are allowed.
So, do you suggest to extend OPTION_FILENAME, and introduce several
macros using it, or a macro taking a bitfield to be filled with
PATH_OPT_FILE, PATH_OPT_DIR, PATH_OPT_EXISTS, PATH_OPT_ABSENT,
PATH_OPT_MASK, PATH_OPT_MODE (require (mode & MASK == MODE))?
Sounds like the big solution to a small problem I had with the word
"file" for a dir...
>> #define OPT_COLOR_FLAG(s, l, v, h) \
>> { OPTION_CALLBACK, (s), (l), (v), N_("when"), (h), PARSE_OPT_OPTARG, \
>> parse_opt_color_flag_cb, (intptr_t)"always" }
next prev parent reply other threads:[~2015-02-24 15:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-23 14:16 [PATCH] checkout: --to requires directory Michael J Gruber
2015-02-23 14:42 ` Jeff King
2015-02-23 15:01 ` Michael J Gruber
2015-02-23 16:17 ` [PATCH 0/4] OPT_{FILENAME,PATH} Michael J Gruber
2015-02-23 16:17 ` [PATCH 1/4] parse-options: introduce OPT_PATH Michael J Gruber
2015-02-23 19:23 ` Junio C Hamano
2015-02-24 15:49 ` Michael J Gruber [this message]
2015-02-23 20:06 ` Philip Oakley
2015-02-23 16:17 ` [PATCH 2/4] option-strings: use OPT_FILENAME Michael J Gruber
2015-02-23 17:44 ` Jeff King
2015-02-23 19:08 ` Junio C Hamano
2015-02-23 20:31 ` Junio C Hamano
2015-02-23 16:17 ` [PATCH 3/4] option-strings: use OPT_PATH Michael J Gruber
2015-02-23 18:26 ` Jeff King
2015-02-23 21:07 ` Junio C Hamano
2015-02-23 21:12 ` Jeff King
2015-02-23 16:17 ` [PATCH 4/4] checkout: --to requires directory Michael J Gruber
2015-02-24 0:34 ` [PATCH] " Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54EC9D85.3010301@drmicha.warpmail.net \
--to=git@drmicha$(echo .)warpmail.net \
--cc=git@vger$(echo .)kernel.org \
--cc=pclouds@gmail$(echo .)com \
--cc=peff@peff$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox