{"thread":{"id":"38633","subject":"[PATCH] checkout: --to requires directory","startedAt":"2015-02-23T14:16:59Z","lastAt":"2015-02-24T15:49:25Z","messageCount":18,"participants":["Michael J Gruber","Jeff King","Junio C Hamano","Philip Oakley","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"256508","messageId":"768eecca8711467b43fcc45403ce9ce91bb1550e.1424700971.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":null,"subject":"[PATCH] checkout: --to requires directory","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T14:16:59Z","receivedAt":"2015-02-23T14:16:59Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"--to requires a directory, not a file. Say so in the usage string.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/checkout.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8b2bf20..8cdcd07 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1341,7 +1341,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"do not limit pathspecs to sparse entries only\")),\n \t\tOPT_HIDDEN_BOOL(0, \"guess\", &dwim_new_local_branch,\n \t\t\t\tN_(\"second guess 'git checkout no-such-branch'\")),\n-\t\tOPT_FILENAME(0, \"to\", &opts.new_worktree,\n+\t\tOPT_STRING(0, \"to\", &opts.new_worktree, N_(\"dir\"),\n \t\t\t N_(\"check a branch out in a separate working directory\")),\n \t\tOPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n \t\t\t N_(\"do not check if another worktree is holding the given ref\")),\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256509","messageId":"20150223144214.GA31624@peff.net","threadId":"38633","inReplyTo":"768eecca8711467b43fcc45403ce9ce91bb1550e.1424700971.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] checkout: --to requires directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-23T14:42:15Z","receivedAt":"2015-02-23T14:42:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2015 at 03:16:59PM +0100, Michael J Gruber wrote:\n\n> --to requires a directory, not a file. Say so in the usage string.\n\nSounds like a good goal, but...\n\n> -\t\tOPT_FILENAME(0, \"to\", &opts.new_worktree,\n> +\t\tOPT_STRING(0, \"to\", &opts.new_worktree, N_(\"dir\"),\n> \t\t\t N_(\"check a branch out in a separate working directory\")),\n\nOPT_FILENAME also calls fix_filename(), which munges the filename so\nthat relative paths given by the user will work, even though git has\nchdir'd to the root of the working tree.\n\nSo you need to handle that somewhere. I think it might be less painful\nto teach OPT_FILENAME to be more flexible in the usage message it\nprints.\n\n-Peff\n"},{"id":"256510","messageId":"54EB40DF.8090800@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"Re: [PATCH] checkout: --to requires directory","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T15:01:51Z","receivedAt":"2015-02-23T15:01:51Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 23.02.2015 15:42:\n> On Mon, Feb 23, 2015 at 03:16:59PM +0100, Michael J Gruber wrote:\n> \n>> --to requires a directory, not a file. Say so in the usage string.\n> \n> Sounds like a good goal, but...\n> \n>> -\t\tOPT_FILENAME(0, \"to\", &opts.new_worktree,\n>> +\t\tOPT_STRING(0, \"to\", &opts.new_worktree, N_(\"dir\"),\n>> \t\t\t N_(\"check a branch out in a separate working directory\")),\n> \n> OPT_FILENAME also calls fix_filename(), which munges the filename so\n> that relative paths given by the user will work, even though git has\n> chdir'd to the root of the working tree.\n\nOoops, I wasn't aware of that. We do use OPT_STRING in quite a few\nplaces for directories, though.\n\n> So you need to handle that somewhere. I think it might be less painful\n> to teach OPT_FILENAME to be more flexible in the usage message it\n> prints.\n\nWe have at least path, template-dir dir which can benefit from that.\n\nAfter Junio's call, I'm trying to look a bit at list-files and multiple\nworkdirs. I guess I should collect those bits.\n\nBTW: multiple workdirs commit messages and doc talk about a hard link\nnamed \"link\" in worktrees//, but I don't get any from \"checkout --to\".\n\nInstead, and in addition to the expected \"gitdir\", I get a file\n\"gitfile\" whose content is a relative path (*not* prefixed with \"gitdir:\n\"), probably to the worktree .git gitfile (relative from the base\nworktree). But \"gitdir\" \"points to\" the same, using (i.e. it contains)\nan absolute path.\n\nI can't quite make sense of this. Maybe the doc is behind? But still.\n\"gitdir\" and \"gitfile\" pointing to the same file.\n\nMichael\n"},{"id":"256518","messageId":"cover.1424707497.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"[PATCH 0/4] OPT_{FILENAME,PATH}","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T16:17:42Z","receivedAt":"2015-02-23T16:17:42Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Here's a \"mini-war\" on file-type arguments being specified as OPT_STRING\n(in order to repent for doing so myself).\n\n1/4 introduces OPT_PATH which is like OPT_FILENAME, but takes an additional\nargument which names the option argument.\n2/4 converts OPT_STRING to OPT_FILENAME where it makes sense\n3/4 converts OPT_STRING to OPT_PATH where it makes sense\n4/4 converts OPT_FILENAME to OPT_PATH for \"checkout --to\"\n\n1/4, 2/4 should apply on top of origin/next independently.\n3/4 needs 1/4.\n4/4 needs 1/4 and is on top of nd/multiple-worktrees.\n\nMichael J Gruber (4):\n parse-options: introduce OPT_PATH\n option-strings: use OPT_FILENAME\n option-strings: use OPT_PATH\n checkout: --to requires directory\n\n Documentation/technical/api-parse-options.txt | 5 +++++\n archive.c | 2 +-\n builtin/archive.c | 2 +-\n builtin/blame.c | 4 ++--\n builtin/checkout.c | 2 +-\n builtin/clone.c | 4 ++--\n builtin/config.c | 2 +-\n builtin/fast-export.c | 4 ++--\n builtin/hash-object.c | 2 +-\n builtin/init-db.c | 4 ++--\n builtin/ls-files.c | 2 +-\n parse-options.h | 2 ++\n 12 files changed, 21 insertions(+), 14 deletions(-)\n\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256516","messageId":"cccf3d14e5582996e5be1467849121a262f9c9f1.1424707497.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"[PATCH 1/4] parse-options: introduce OPT_PATH","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T16:17:43Z","receivedAt":"2015-02-23T16:17:43Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Many options are paths, but not files. Introduce OPT_PATH which does\nthe same path processing as OPT_FILENAME but allows to name the argument.\n\nSigned-off-by: Michael J Gruber \n---\n Documentation/technical/api-parse-options.txt | 5 +++++\n parse-options.h | 2 ++\n 2 files changed, 7 insertions(+)\n\ndiff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt\nindex 1f2db31..109903c 100644\n--- a/Documentation/technical/api-parse-options.txt\n+++ b/Documentation/technical/api-parse-options.txt\n@@ -187,6 +187,11 @@ There are some macros to easily define options:\n \tThe filename will be prefixed by passing the filename along with\n \tthe prefix argument of `parse_options()` to `prefix_filename()`.\n \n+`OPT_PATH(short, long, &var, arg_str, description)`::\n+\tIntroduce an option with a path argument named arg_str.\n+\tThe filename will be prefixed by passing the filename along with\n+\tthe prefix argument of `parse_options()` to `prefix_filename()`.\n+\n `OPT_ARGUMENT(long, description)`::\n \tIntroduce a long-option argument that will be kept in `argv[]`.\n \ndiff --git a/parse-options.h b/parse-options.h\nindex 7940bc7..5127a5d 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -149,6 +149,8 @@ struct option {\n \t PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }\n #define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \\\n \t\t\t\t N_(\"file\"), (h) }\n+#define OPT_PATH(s, l, v, a, h) { OPTION_FILENAME, (s), (l), (v), \\\n+\t\t\t\t (a), (h) }\n #define OPT_COLOR_FLAG(s, l, v, h) \\\n \t{ OPTION_CALLBACK, (s), (l), (v), N_(\"when\"), (h), PARSE_OPT_OPTARG, \\\n \t\tparse_opt_color_flag_cb, (intptr_t)\"always\" }\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256515","messageId":"3af5c93959b22dc327d7fb3974d36764906c2969.1424707497.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"[PATCH 2/4] option-strings: use OPT_FILENAME","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T16:17:44Z","receivedAt":"2015-02-23T16:17:44Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Some commands use OPT_STRING to specify a argument. Let them use\nOPT_FILENAME so that they can profit from path prefixing.\n\nThis excludes low-level commands like the credential helpers.\n\nSigned-off-by: Michael J Gruber \n---\nThis could be before 1/4 but seemed more logical here.\n\n archive.c | 2 +-\n builtin/archive.c | 2 +-\n builtin/blame.c | 4 ++--\n builtin/config.c | 2 +-\n builtin/fast-export.c | 4 ++--\n builtin/hash-object.c | 2 +-\n builtin/ls-files.c | 2 +-\n 7 files changed, 9 insertions(+), 9 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 94a9981..b4da255 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -419,7 +419,7 @@ static int parse_archive_args(int argc, const char **argv,\n \t\tOPT_STRING(0, \"format\", &format, N_(\"fmt\"), N_(\"archive format\")),\n \t\tOPT_STRING(0, \"prefix\", &base, N_(\"prefix\"),\n \t\t\tN_(\"prepend prefix to each pathname in the archive\")),\n-\t\tOPT_STRING('o', \"output\", &output, N_(\"file\"),\n+\t\tOPT_FILENAME('o', \"output\", &output,\n \t\t\tN_(\"write the archive to this file\")),\n \t\tOPT_BOOL(0, \"worktree-attributes\", &worktree_attributes,\n \t\t\tN_(\"read .gitattributes in working directory\")),\ndiff --git a/builtin/archive.c b/builtin/archive.c\nindex a1e3b94..9c96681 100644\n--- a/builtin/archive.c\n+++ b/builtin/archive.c\n@@ -85,7 +85,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n \tconst char *output = NULL;\n \tconst char *remote = NULL;\n \tstruct option local_opts[] = {\n-\t\tOPT_STRING('o', \"output\", &output, N_(\"file\"),\n+\t\tOPT_FILENAME('o', \"output\", &output,\n \t\t\tN_(\"write the archive to this file\")),\n \t\tOPT_STRING(0, \"remote\", &remote, N_(\"repo\"),\n \t\t\tN_(\"retrieve the archive from remote repository \")),\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 303e217..9b14c7c 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -2514,8 +2514,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"Show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"Ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n-\t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"Use revisions from instead of calling git-rev-list\")),\n-\t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"Use 's contents as the final image\")),\n+\t\tOPT_FILENAME('S', NULL, &revs_file, N_(\"Use revisions from instead of calling git-rev-list\")),\n+\t\tOPT_FILENAME(0, \"contents\", &contents_from, N_(\"Use 's contents as the final image\")),\n \t\t{ OPTION_CALLBACK, 'C', NULL, &opt, N_(\"score\"), N_(\"Find line copies within and across files\"), PARSE_OPT_OPTARG, blame_copy_callback },\n \t\t{ OPTION_CALLBACK, 'M', NULL, &opt, N_(\"score\"), N_(\"Find line movements within and across files\"), PARSE_OPT_OPTARG, blame_move_callback },\n \t\tOPT_STRING_LIST('L', NULL, &range_list, N_(\"n,m\"), N_(\"Process only line range n,m, counting from 1\")),\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 8cc2604..b80922c 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -54,7 +54,7 @@ static struct option builtin_config_options[] = {\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n-\tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n+\tOPT_FILENAME('f', \"file\", &given_config_source.file, N_(\"use given config file\")),\n \tOPT_STRING(0, \"blob\", &given_config_source.blob, N_(\"blob-id\"), N_(\"read config from given blob object\")),\n \tOPT_GROUP(N_(\"Action\")),\n \tOPT_BIT(0, \"get\", &actions, N_(\"get value: name [value-regex]\"), ACTION_GET),\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex b8182c2..44f2600 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -983,9 +983,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n \t\tOPT_CALLBACK(0, \"tag-of-filtered-object\", &tag_of_filtered_mode, N_(\"mode\"),\n \t\t\t N_(\"select handling of tags that tag filtered objects\"),\n \t\t\t parse_opt_tag_of_filtered_mode),\n-\t\tOPT_STRING(0, \"export-marks\", &export_filename, N_(\"file\"),\n+\t\tOPT_FILENAME(0, \"export-marks\", &export_filename,\n \t\t\t N_(\"Dump marks to this file\")),\n-\t\tOPT_STRING(0, \"import-marks\", &import_filename, N_(\"file\"),\n+\t\tOPT_FILENAME(0, \"import-marks\", &import_filename,\n \t\t\t N_(\"Import marks from this file\")),\n \t\tOPT_BOOL(0, \"fake-missing-tagger\", &fake_missing_tagger,\n \t\t\t N_(\"Fake a tagger when tags lack one\")),\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex 6158363..7b13940 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -98,7 +98,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n \t\tOPT_BOOL( 0 , \"stdin-paths\", &stdin_paths, N_(\"read file names from stdin\")),\n \t\tOPT_BOOL( 0 , \"no-filters\", &no_filters, N_(\"store file as is without filters\")),\n \t\tOPT_BOOL( 0, \"literally\", &literally, N_(\"just hash any random garbage to create corrupt objects for debugging Git\")),\n-\t\tOPT_STRING( 0 , \"path\", &vpath, N_(\"file\"), N_(\"process file as it were from this path\")),\n+\t\tOPT_FILENAME( 0 , \"path\", &vpath, N_(\"process file as it were from this path\")),\n \t\tOPT_END()\n \t};\n \tint i;\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 99cee20..0ddd3a8 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -489,7 +489,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n \t\t{ OPTION_CALLBACK, 'X', \"exclude-from\", &dir, N_(\"file\"),\n \t\t\tN_(\"exclude patterns are read from \"),\n \t\t\t0, option_parse_exclude_from },\n-\t\tOPT_STRING(0, \"exclude-per-directory\", &dir.exclude_per_dir, N_(\"file\"),\n+\t\tOPT_FILENAME(0, \"exclude-per-directory\", &dir.exclude_per_dir,\n \t\t\tN_(\"read additional per-directory exclude patterns in \")),\n \t\t{ OPTION_CALLBACK, 0, \"exclude-standard\", &dir, NULL,\n \t\t\tN_(\"add the standard git exclusions\"),\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256517","messageId":"9030a1bb1e7a03eedfd65432fabf385f457419d6.1424707497.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"[PATCH 3/4] option-strings: use OPT_PATH","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T16:17:45Z","receivedAt":"2015-02-23T16:17:45Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Some commands use OPT_STRING to specify a path type argument. Let them\nuse OPT_PATH so that they can profit from path prefixing.\n\nSigned-off-by: Michael J Gruber \n---\n builtin/clone.c | 4 ++--\n builtin/init-db.c | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex a9af3f2..15941c5 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -80,7 +80,7 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"initialize submodules in the clone\")),\n \tOPT_BOOL(0, \"recurse-submodules\", &option_recursive,\n \t\t N_(\"initialize submodules in the clone\")),\n-\tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n+\tOPT_PATH(0, \"template\", &option_template, N_(\"template-directory\"),\n \t\t N_(\"directory from which templates will be used\")),\n \tOPT_CALLBACK(0 , \"reference\", &option_reference, N_(\"repo\"),\n \t\t N_(\"reference repository\"), &opt_parse_reference),\n@@ -94,7 +94,7 @@ static struct option builtin_clone_options[] = {\n \t\t N_(\"create a shallow clone of that depth\")),\n \tOPT_BOOL(0, \"single-branch\", &option_single_branch,\n \t\t N_(\"clone only one branch, HEAD or --branch\")),\n-\tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n+\tOPT_PATH(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n \t\t N_(\"separate git dir from working tree\")),\n \tOPT_STRING_LIST('c', \"config\", &option_config, N_(\"key=value\"),\n \t\t\tN_(\"set config inside the new repository\")),\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 6b7fa5f..262c9ae 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -477,7 +477,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \tconst char *template_dir = NULL;\n \tunsigned int flags = 0;\n \tconst struct option init_db_options[] = {\n-\t\tOPT_STRING(0, \"template\", &template_dir, N_(\"template-directory\"),\n+\t\tOPT_PATH(0, \"template\", &template_dir, N_(\"template-directory\"),\n \t\t\t\tN_(\"directory from which templates will be used\")),\n \t\tOPT_SET_INT(0, \"bare\", &is_bare_repository_cfg,\n \t\t\t\tN_(\"create a bare repository\"), 1),\n@@ -486,7 +486,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\tN_(\"specify that the git repository is to be shared amongst several users\"),\n \t\t\tPARSE_OPT_OPTARG | PARSE_OPT_NONEG, shared_callback, 0},\n \t\tOPT_BIT('q', \"quiet\", &flags, N_(\"be quiet\"), INIT_DB_QUIET),\n-\t\tOPT_STRING(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n+\t\tOPT_PATH(0, \"separate-git-dir\", &real_git_dir, N_(\"gitdir\"),\n \t\t\t N_(\"separate git dir from working tree\")),\n \t\tOPT_END()\n \t};\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256519","messageId":"3ae582f2ca4e4a7ef6cb0041148037bbb31c5e09.1424707497.git.git@drmicha.warpmail.net","threadId":"38633","inReplyTo":"20150223144214.GA31624@peff.net","subject":"[PATCH 4/4] checkout: --to requires directory","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-23T16:17:46Z","receivedAt":"2015-02-23T16:17:46Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"--to requires a directory, not a file. Say so in the usage string.\n\nSigned-off-by: Michael J Gruber \n---\nI did not spot any other \"misuses\" of OPT_FILENAME (for non-files).\n\n builtin/checkout.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 8b2bf20..4256560 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1341,7 +1341,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"do not limit pathspecs to sparse entries only\")),\n \t\tOPT_HIDDEN_BOOL(0, \"guess\", &dwim_new_local_branch,\n \t\t\t\tN_(\"second guess 'git checkout no-such-branch'\")),\n-\t\tOPT_FILENAME(0, \"to\", &opts.new_worktree,\n+\t\tOPT_PATH(0, \"to\", &opts.new_worktree, N_(\"dir\"),\n \t\t\t N_(\"check a branch out in a separate working directory\")),\n \t\tOPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n \t\t\t N_(\"do not check if another worktree is holding the given ref\")),\n-- \n2.3.0.296.g32c87e1\n"},{"id":"256524","messageId":"20150223174430.GA19904@peff.net","threadId":"38633","inReplyTo":"3af5c93959b22dc327d7fb3974d36764906c2969.1424707497.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/4] option-strings: use OPT_FILENAME","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-23T17:44:30Z","receivedAt":"2015-02-23T17:44:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2015 at 05:17:44PM +0100, Michael J Gruber wrote:\n\n> diff --git a/archive.c b/archive.c\n> index 94a9981..b4da255 100644\n> --- a/archive.c\n> +++ b/archive.c\n> @@ -419,7 +419,7 @@ static int parse_archive_args(int argc, const char **argv,\n> \t\tOPT_STRING(0, \"format\", &format, N_(\"fmt\"), N_(\"archive format\")),\n> \t\tOPT_STRING(0, \"prefix\", &base, N_(\"prefix\"),\n> \t\t\tN_(\"prepend prefix to each pathname in the archive\")),\n> -\t\tOPT_STRING('o', \"output\", &output, N_(\"file\"),\n> +\t\tOPT_FILENAME('o', \"output\", &output,\n> \t\t\tN_(\"write the archive to this file\")),\n> \t\tOPT_BOOL(0, \"worktree-attributes\", &worktree_attributes,\n> \t\t\tN_(\"read .gitattributes in working directory\")),\n\nThis one is rather tricky...if you look down a few lines, you will see\nthat we just die() when the option is specified. The point is that\n--output is not supposed to make it down to this level (it gets\nintercepted by cmd_archive first), and we would not want to respect it\nfor a remote invocation (ditto for --exec and --remote options).\n\nIt's a little weird that we have it here at all (after all, if we\nomitted it, we would _also_ complain here). But I guess the \"-h\" text is\ngenerated by this parse_options invocation. That seems to be confirmed\nby the commit messages of 4fac1d3a98 and 52e7787609.\n\nSo I think your change has literally no impact; we do not care about\nprefixing or not here, and the help text remains the same (because it\nalready said \"file\"). I am not sure which is better from a readability\nstandpoint. On the one hand, it is clear that we should match the option\nin cmd_archive, since we are printing the help for it here. But we would\nnever want to act in any way on the filename-properties of this string\n(e.g., if we ever started looking at the filesystem as part of\nfix_filename(), this would leak information if a malicious client sent\n\"--output=foo\" to a git-upload-archive server). That's probably a rather\nunlikely scenario, though.\n\n> diff --git a/builtin/archive.c b/builtin/archive.c\n> index a1e3b94..9c96681 100644\n> --- a/builtin/archive.c\n> +++ b/builtin/archive.c\n> @@ -85,7 +85,7 @@ int cmd_archive(int argc, const char **argv, const char *prefix)\n> \tconst char *output = NULL;\n> \tconst char *remote = NULL;\n> \tstruct option local_opts[] = {\n> -\t\tOPT_STRING('o', \"output\", &output, N_(\"file\"),\n> +\t\tOPT_FILENAME('o', \"output\", &output,\n> \t\t\tN_(\"write the archive to this file\")),\n> \t\tOPT_STRING(0, \"remote\", &remote, N_(\"repo\"),\n> \t\t\tN_(\"retrieve the archive from remote repository \")),\n\nSo this one is the flip-side of the parse_archive_args() change above.\nAny changes to the help-string here will _not_ get shown (we use\nPARSE_OPT_KEEP_ALL, which implies PARSE_OPT_NO_INTERNAL_HELP, and the\n\"-h\" gets passed down to parse_archive_args parser).\n\nBut it should be changing the behavior of \"-o\" to properly handle\nrelative paths, which is a good thing. Except it doesn't. :)\n\nThe entry in git.c:commands for cmd_archive does not mention\n\"RUN_SETUP\", so we will not have actually chdir'd here. It already works\nfine.\n\nI do not think using OPT_FILENAME here is _hurting_ anything, though.\nThe \"prefix\" variable will always be NULL, and therefore fix_filename()\nwill be a noop. And the code is hopefully more obvious as a result. So\nI'd be OK with this change, even though it technically does nothing.\n\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index 303e217..9b14c7c 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -2514,8 +2514,8 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BIT('e', \"show-email\", &output_option, N_(\"Show author email instead of name (Default: off)\"), OUTPUT_SHOW_EMAIL),\n> \t\tOPT_BIT('w', NULL, &xdl_opts, N_(\"Ignore whitespace differences\"), XDF_IGNORE_WHITESPACE),\n> \t\tOPT_BIT(0, \"minimal\", &xdl_opts, N_(\"Spend extra cycles to find better match\"), XDF_NEED_MINIMAL),\n> -\t\tOPT_STRING('S', NULL, &revs_file, N_(\"file\"), N_(\"Use revisions from instead of calling git-rev-list\")),\n> -\t\tOPT_STRING(0, \"contents\", &contents_from, N_(\"file\"), N_(\"Use 's contents as the final image\")),\n> +\t\tOPT_FILENAME('S', NULL, &revs_file, N_(\"Use revisions from instead of calling git-rev-list\")),\n> +\t\tOPT_FILENAME(0, \"contents\", &contents_from, N_(\"Use 's contents as the final image\")),\n\nThese ones _are_ actually doing something, and it seems like a strict\nimprovement. E.g.:\n\n cd t &&\n git blame --contents=test-lib.sh test-lib.sh\n\ndoes not work without your patch.\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index 8cc2604..b80922c 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -54,7 +54,7 @@ static struct option builtin_config_options[] = {\n> \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n> \tOPT_BOOL(0, \"system\", &use_system_config, N_(\"use system config file\")),\n> \tOPT_BOOL(0, \"local\", &use_local_config, N_(\"use repository config file\")),\n> -\tOPT_STRING('f', \"file\", &given_config_source.file, N_(\"file\"), N_(\"use given config file\")),\n> +\tOPT_FILENAME('f', \"file\", &given_config_source.file, N_(\"use given config file\")),\n> \tOPT_STRING(0, \"blob\", &given_config_source.blob, N_(\"blob-id\"), N_(\"read config from given blob object\")),\n> \tOPT_GROUP(N_(\"Action\")),\n> \tOPT_BIT(0, \"get\", &actions, N_(\"get value: name [value-regex]\"), ACTION_GET),\n\nThis one introduces a bug. We already handle prepending the prefix to\nthe filename later in cmd_config(). Doing:\n\n cd t &&\n git config --file=../.git/config --list\n\nworks, but with your patch produces:\n\n fatal: unable to read config file 't/t/../.git/config': No such file or directory\n\nIn theory we can get rid of the manual handling in cmd_config in favor\nof using OPT_FILENAME. There are some spots where we assign to\ngiven_config_source.file after parsing options but before doing the\nfixup, but I think they should all be absolute paths (they come from\nhome_config_paths(), so unless you have a relative home directory...).\n\nIt might be worth double-checking the test coverage before touching this\narea too much, though.\n\n> diff --git a/builtin/fast-export.c b/builtin/fast-export.c\n> index b8182c2..44f2600 100644\n> --- a/builtin/fast-export.c\n> +++ b/builtin/fast-export.c\n> @@ -983,9 +983,9 @@ int cmd_fast_export(int argc, const char **argv, const char *prefix)\n> \t\tOPT_CALLBACK(0, \"tag-of-filtered-object\", &tag_of_filtered_mode, N_(\"mode\"),\n> \t\t\t N_(\"select handling of tags that tag filtered objects\"),\n> \t\t\t parse_opt_tag_of_filtered_mode),\n> -\t\tOPT_STRING(0, \"export-marks\", &export_filename, N_(\"file\"),\n> +\t\tOPT_FILENAME(0, \"export-marks\", &export_filename,\n> \t\t\t N_(\"Dump marks to this file\")),\n> -\t\tOPT_STRING(0, \"import-marks\", &import_filename, N_(\"file\"),\n> +\t\tOPT_FILENAME(0, \"import-marks\", &import_filename,\n> \t\t\t N_(\"Import marks from this file\")),\n> \t\tOPT_BOOL(0, \"fake-missing-tagger\", &fake_missing_tagger,\n> \t\t\t N_(\"Fake a tagger when tags lack one\")),\n\nThese ones look like a straightforward improvement.\n\n> diff --git a/builtin/hash-object.c b/builtin/hash-object.c\n> index 6158363..7b13940 100644\n> --- a/builtin/hash-object.c\n> +++ b/builtin/hash-object.c\n> @@ -98,7 +98,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n> \t\tOPT_BOOL( 0 , \"stdin-paths\", &stdin_paths, N_(\"read file names from stdin\")),\n> \t\tOPT_BOOL( 0 , \"no-filters\", &no_filters, N_(\"store file as is without filters\")),\n> \t\tOPT_BOOL( 0, \"literally\", &literally, N_(\"just hash any random garbage to create corrupt objects for debugging Git\")),\n> -\t\tOPT_STRING( 0 , \"path\", &vpath, N_(\"file\"), N_(\"process file as it were from this path\")),\n> +\t\tOPT_FILENAME( 0 , \"path\", &vpath, N_(\"process file as it were from this path\")),\n> \t\tOPT_END()\n\nI'm not sure about this one. hash-object does not do SETUP_GIT\nautomatically, so at the time of the parsing, our prefix is empty. So\nit's always a noop.\n\nLater, we _do_ tack the prefix on here, but only after we have called\nsetup_git_directory (and only if we are writing out the object, since\notherwise we do not need to be in a .git directory at all).\n\nSo using OPT_FILENAME here doesn't produce any wrong behavior, but it is\npotentially quite confusing.\n\n> diff --git a/builtin/ls-files.c b/builtin/ls-files.c\n> index 99cee20..0ddd3a8 100644\n> --- a/builtin/ls-files.c\n> +++ b/builtin/ls-files.c\n> @@ -489,7 +489,7 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)\n> \t\t{ OPTION_CALLBACK, 'X', \"exclude-from\", &dir, N_(\"file\"),\n> \t\t\tN_(\"exclude patterns are read from \"),\n> \t\t\t0, option_parse_exclude_from },\n> -\t\tOPT_STRING(0, \"exclude-per-directory\", &dir.exclude_per_dir, N_(\"file\"),\n> +\t\tOPT_FILENAME(0, \"exclude-per-directory\", &dir.exclude_per_dir,\n> \t\t\tN_(\"read additional per-directory exclude patterns in \")),\n> \t\t{ OPTION_CALLBACK, 0, \"exclude-standard\", &dir, NULL,\n> \t\t\tN_(\"add the standard git exclusions\"),\n\nI don't think this is right. The per-directory exclude path gets tacked\nonto each directory we traverse. It is not a single path in itself, and\nprepending our current prefix to it would definitely be the wrong thing\n(if you were in \"t/\", we would not want to look for \"Documentation/t/.gitignore\").\n\n-Peff\n"},{"id":"256527","messageId":"20150223182653.GB19904@peff.net","threadId":"38633","inReplyTo":"9030a1bb1e7a03eedfd65432fabf385f457419d6.1424707497.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 3/4] option-strings: use OPT_PATH","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-23T18:26:53Z","receivedAt":"2015-02-23T18:26:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2015 at 05:17:45PM +0100, Michael J Gruber wrote:\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index a9af3f2..15941c5 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -80,7 +80,7 @@ static struct option builtin_clone_options[] = {\n> \t\t N_(\"initialize submodules in the clone\")),\n> \tOPT_BOOL(0, \"recurse-submodules\", &option_recursive,\n> \t\t N_(\"initialize submodules in the clone\")),\n> -\tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n> +\tOPT_PATH(0, \"template\", &option_template, N_(\"template-directory\"),\n> \t\t N_(\"directory from which templates will be used\")),\n> \tOPT_CALLBACK(0 , \"reference\", &option_reference, N_(\"repo\"),\n> \t\t N_(\"reference repository\"), &opt_parse_reference),\n\nI'm not sure if this one is doing anything. Clone cannot use SETUP_GIT\nfor obvious reasons, so we should have a NULL prefix here. But that also\nmeans we should be doing the right thing already.\n\nI think the same goes for the rest of the instances in this patch.\n\n-Peff\n"},{"id":"256534","messageId":"xmqqfv9wtpha.fsf@gitster.dls.corp.google.com","threadId":"38633","inReplyTo":"3af5c93959b22dc327d7fb3974d36764906c2969.1424707497.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/4] option-strings: use OPT_FILENAME","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-23T19:08:49Z","receivedAt":"2015-02-23T19:08:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Some commands use OPT_STRING to specify a argument. Let them use\n> OPT_FILENAME so that they can profit from path prefixing.\n\nIf the existing code that takes string actually already works\ncorrectly, they must be doing the necessary prefixing on their own,\nbut I do not see in this patch a hunk that removes such a custom\nprefixing. Puzzled...\n\nIt may be that some of them do not work correctly from subdirectory\nand this change fixes bugs, but are all of them like that, or is\nthis patch fixing some of them while breaking some others?\n>\n> archive.c | 2 +-\n> builtin/archive.c | 2 +-\n> builtin/blame.c | 4 ++--\n> builtin/config.c | 2 +-\n> builtin/fast-export.c | 4 ++--\n> builtin/hash-object.c | 2 +-\n> builtin/ls-files.c | 2 +-\n> 7 files changed, 9 insertions(+), 9 deletions(-)\n"},{"id":"256536","messageId":"xmqqbnkktoti.fsf@gitster.dls.corp.google.com","threadId":"38633","inReplyTo":"cccf3d14e5582996e5be1467849121a262f9c9f1.1424707497.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/4] parse-options: introduce OPT_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-23T19:23:05Z","receivedAt":"2015-02-23T19:23:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber writes:\n\n> Many options are paths, but not files. Introduce OPT_PATH which does\n> the same path processing as OPT_FILENAME but allows to name the argument.\n> ...\n> diff --git a/parse-options.h b/parse-options.h\n> index 7940bc7..5127a5d 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -149,6 +149,8 @@ struct option {\n> \t PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }\n> #define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \\\n> \t\t\t\t N_(\"file\"), (h) }\n> +#define OPT_PATH(s, l, v, a, h) { OPTION_FILENAME, (s), (l), (v), \\\n> +\t\t\t\t (a), (h) }\n\nI am somewhat disappointed to see this implementation.\n\n - I expected that OPT_FILENAME will be re-implemented in terms of\n OPT_PATH(), as a thin wrapper.\n\n - As the original complaint was \"checkout --to requires a\n directory, and a file would not work\", I expected this to give\n the programmer finer controls, such as:\n\n - The name must refer to an existing entity on the filesystem,\n or an existing entity on the filesystem must not exist, or\n anything is fine (tristate).\n\n - The name refers to a diretory, or the name refers to a regular\n file, or the name refers to a symbolic link, or anything goes.\n\nThat is merely \"somewhat\", as the latter _can_ be coded (e.g. if you\ncare that the given path already exists as a directory, stat() it\nand see if it is, or if you want that the given path does not exist\nyet, stat() it to make sure you get ENOENT) after the option is\nparsed by the program that uses the parser.\n\nBut the infrastructure to allow the latter would free you from\nhaving to say N_(\"file\") or N_(\"directory\"); if a parameter can\nrefer to either a file or a directory, the parse-options library\ncould give you N_(\"file or directory\") because you are already\ntelling what kind(s) of paths are allowed.\n\n> #define OPT_COLOR_FLAG(s, l, v, h) \\\n> \t{ OPTION_CALLBACK, (s), (l), (v), N_(\"when\"), (h), PARSE_OPT_OPTARG, \\\n> \t\tparse_opt_color_flag_cb, (intptr_t)\"always\" }\n"},{"id":"256540","messageId":"6D98922D49BD42139185BE6A94DDD5DA@PhilipOakley","threadId":"38633","inReplyTo":"cccf3d14e5582996e5be1467849121a262f9c9f1.1424707497.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 1/4] parse-options: introduce OPT_PATH","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2015-02-23T19:45:30Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Michael J Gruber\" \n> Many options are paths, but not files. Introduce OPT_PATH which does\n> the same path processing as OPT_FILENAME but allows to name the \n> argument.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> Documentation/technical/api-parse-options.txt | 5 +++++\n> parse-options.h | 2 ++\n> 2 files changed, 7 insertions(+)\n>\n> diff --git a/Documentation/technical/api-parse-options.txt \n> b/Documentation/technical/api-parse-options.txt\n> index 1f2db31..109903c 100644\n> --- a/Documentation/technical/api-parse-options.txt\n> +++ b/Documentation/technical/api-parse-options.txt\n> @@ -187,6 +187,11 @@ There are some macros to easily define options:\n> The filename will be prefixed by passing the filename along with\n> the prefix argument of `parse_options()` to `prefix_filename()`.\n>\n> +`OPT_PATH(short, long, &var, arg_str, description)`::\n> + Introduce an option with a path argument named arg_str.\n> + The filename will be prefixed by passing the filename along with\n\nI couldn't understand this. Should it be s/filename/pathname/ ? Or am I \nway off topic.\n\n> + the prefix argument of `parse_options()` to `prefix_filename()`.\n> +\n> `OPT_ARGUMENT(long, description)`::\n> Introduce a long-option argument that will be kept in `argv[]`.\n>\n> diff --git a/parse-options.h b/parse-options.h\n> index 7940bc7..5127a5d 100644\n> --- a/parse-options.h\n> +++ b/parse-options.h\n> @@ -149,6 +149,8 @@ struct option {\n> PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }\n> #define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \n> \\\n> N_(\"file\"), (h) }\n> +#define OPT_PATH(s, l, v, a, h) { OPTION_FILENAME, (s), (l), (v), \n> \\\n> + (a), (h) }\n> #define OPT_COLOR_FLAG(s, l, v, h) \\\n> { OPTION_CALLBACK, (s), (l), (v), N_(\"when\"), (h), PARSE_OPT_OPTARG, \n> \\\n> parse_opt_color_flag_cb, (intptr_t)\"always\" }\n> -- \n> 2.3.0.296.g32c87e1\n>\n> --\nPhilip \n"},{"id":"256546","messageId":"xmqqioess739.fsf@gitster.dls.corp.google.com","threadId":"38633","inReplyTo":"xmqqfv9wtpha.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 2/4] option-strings: use OPT_FILENAME","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-23T20:31:22Z","receivedAt":"2015-02-23T20:31:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Michael J Gruber writes:\n>\n>> Some commands use OPT_STRING to specify a argument. Let them use\n>> OPT_FILENAME so that they can profit from path prefixing.\n>\n> If the existing code that takes string actually already works\n> correctly, they must be doing the necessary prefixing on their own,\n> but I do not see in this patch a hunk that removes such a custom\n> prefixing. Puzzled...\n>\n> It may be that some of them do not work correctly from subdirectory\n> and this change fixes bugs, but are all of them like that, or is\n> this patch fixing some of them while breaking some others?\n\nAhh, I see Peff already went over the changes in the patch and\nsifted them into good, questionable and bad bins.\n\nThanks; I'll shut up and await for a reroll ;-)\n"},{"id":"256547","messageId":"xmqqd250s5fi.fsf@gitster.dls.corp.google.com","threadId":"38633","inReplyTo":"20150223182653.GB19904@peff.net","subject":"Re: [PATCH 3/4] option-strings: use OPT_PATH","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-02-23T21:07:13Z","receivedAt":"2015-02-23T21:07:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Feb 23, 2015 at 05:17:45PM +0100, Michael J Gruber wrote:\n>\n>> diff --git a/builtin/clone.c b/builtin/clone.c\n>> index a9af3f2..15941c5 100644\n>> --- a/builtin/clone.c\n>> +++ b/builtin/clone.c\n>> @@ -80,7 +80,7 @@ static struct option builtin_clone_options[] = {\n>> \t\t N_(\"initialize submodules in the clone\")),\n>> \tOPT_BOOL(0, \"recurse-submodules\", &option_recursive,\n>> \t\t N_(\"initialize submodules in the clone\")),\n>> -\tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n>> +\tOPT_PATH(0, \"template\", &option_template, N_(\"template-directory\"),\n>> \t\t N_(\"directory from which templates will be used\")),\n>> \tOPT_CALLBACK(0 , \"reference\", &option_reference, N_(\"repo\"),\n>> \t\t N_(\"reference repository\"), &opt_parse_reference),\n>\n> I'm not sure if this one is doing anything. Clone cannot use SETUP_GIT\n> for obvious reasons, so we should have a NULL prefix here. But that also\n> means we should be doing the right thing already.\n\nI somehow thought that OPT_FILENAME already used expand_user_path()\nbut apparently it does not. It may want to.\n\nAnd then this change will start to matter, as a good enhancement.\n\nOf course, if OPT_PATH() is introduced in such a way that the\nprogram that uses the API can ask for \"existing\" and/or \"directory\",\n\n git clone --template=existing-file $URL\n git clone --template=no-such-directory $URL\n\ncan be diagnosed as an error without the program having to code very\nmuch.\n\nSo, I agree that this change does not do anything in the current\ncodebase, but it goes in a right direction.\n"},{"id":"256548","messageId":"20150223211228.GA6658@peff.net","threadId":"38633","inReplyTo":"xmqqd250s5fi.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 3/4] option-strings: use OPT_PATH","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-02-23T21:12:28Z","receivedAt":"2015-02-23T21:12:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 23, 2015 at 01:07:13PM -0800, Junio C Hamano wrote:\n\n> >> -\tOPT_STRING(0, \"template\", &option_template, N_(\"template-directory\"),\n> >> +\tOPT_PATH(0, \"template\", &option_template, N_(\"template-directory\"),\n> >> \t\t N_(\"directory from which templates will be used\")),\n> >> \tOPT_CALLBACK(0 , \"reference\", &option_reference, N_(\"repo\"),\n> >> \t\t N_(\"reference repository\"), &opt_parse_reference),\n> >\n> > I'm not sure if this one is doing anything. Clone cannot use SETUP_GIT\n> > for obvious reasons, so we should have a NULL prefix here. But that also\n> > means we should be doing the right thing already.\n> \n> I somehow thought that OPT_FILENAME already used expand_user_path()\n> but apparently it does not. It may want to.\n> \n> And then this change will start to matter, as a good enhancement.\n> \n> Of course, if OPT_PATH() is introduced in such a way that the\n> program that uses the API can ask for \"existing\" and/or \"directory\",\n> \n> git clone --template=existing-file $URL\n> git clone --template=no-such-directory $URL\n> \n> can be diagnosed as an error without the program having to code very\n> much.\n> \n> So, I agree that this change does not do anything in the current\n> codebase, but it goes in a right direction.\n\nOh, I agree that annotating the option with more meaning is not a bad\nthing. It may help readability, and as you note, we may grow new\nfeatures on those annotations over time. I mostly just got tired of\nwriting things along those lines by the time I finished with the\nOPT_FILENAME patch (I guess OPT_PATH is the one more likely to grow new\nfeatures, though).\n\nThe one exception here is the thing I mentioned in parse_archive_args:\nthat one can be controlled by a remote caller and we would not want to\nleak any information about which files exist, where the user's home\ndirectory is, etc.\n\n-Peff\n"},{"id":"256552","messageId":"CACsJy8Bn_cOGzNfPvaqwPztH3==+EVvN8eciYApMc36epbEz1g@mail.gmail.com","threadId":"38633","inReplyTo":"768eecca8711467b43fcc45403ce9ce91bb1550e.1424700971.git.git@drmicha.warpmail.net","subject":"Re: [PATCH] checkout: --to requires directory","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-02-24T00:34:31Z","receivedAt":"2015-02-24T00:34:31Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Feb 23, 2015 at 9:16 PM, Michael J Gruber\n wrote:\n> --to requires a directory, not a file. Say so in the usage string.\n>\n> Signed-off-by: Michael J Gruber \n> ---\n> builtin/checkout.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/builtin/checkout.c b/builtin/checkout.c\n> index 8b2bf20..8cdcd07 100644\n> --- a/builtin/checkout.c\n> +++ b/builtin/checkout.c\n> @@ -1341,7 +1341,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix)\n> N_(\"do not limit pathspecs to sparse entries only\")),\n> OPT_HIDDEN_BOOL(0, \"guess\", &dwim_new_local_branch,\n> N_(\"second guess 'git checkout no-such-branch'\")),\n> - OPT_FILENAME(0, \"to\", &opts.new_worktree,\n> + OPT_STRING(0, \"to\", &opts.new_worktree, N_(\"dir\"),\n\nNack. OPT_FILENAME has hidden magic, see fix_filename(). If you want\nto change the text, you'll have to fall back to using OPTION_FILENAME\ndirectly.\n\n> N_(\"check a branch out in a separate working directory\")),\n> OPT_BOOL(0, \"ignore-other-worktrees\", &opts.ignore_other_worktrees,\n> N_(\"do not check if another worktree is holding the given ref\")),\n\n\n\n-- \nDuy\n"},{"id":"256579","messageId":"54EC9D85.3010301@drmicha.warpmail.net","threadId":"38633","inReplyTo":"xmqqbnkktoti.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/4] parse-options: introduce OPT_PATH","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-02-24T15:49:25Z","receivedAt":"2015-02-24T15:49:25Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 23.02.2015 20:23:\n> Michael J Gruber writes:\n> \n>> Many options are paths, but not files. Introduce OPT_PATH which does\n>> the same path processing as OPT_FILENAME but allows to name the argument.\n>> ...\n>> diff --git a/parse-options.h b/parse-options.h\n>> index 7940bc7..5127a5d 100644\n>> --- a/parse-options.h\n>> +++ b/parse-options.h\n>> @@ -149,6 +149,8 @@ struct option {\n>> \t PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }\n>> #define OPT_FILENAME(s, l, v, h) { OPTION_FILENAME, (s), (l), (v), \\\n>> \t\t\t\t N_(\"file\"), (h) }\n>> +#define OPT_PATH(s, l, v, a, h) { OPTION_FILENAME, (s), (l), (v), \\\n>> +\t\t\t\t (a), (h) }\n> \n> I am somewhat disappointed to see this implementation.\n> \n> - I expected that OPT_FILENAME will be re-implemented in terms of\n> OPT_PATH(), as a thin wrapper.\n\nRight now they are just two macros. Would\n\n#define OPT_FILENAME(s, l, v, h) OPT_PATH((s), (l), (v), N_(\"file\"), (h))\n\nbe more what you expect? I don't consider that thinner but don't mind\neither way.\n\n> - As the original complaint was \"checkout --to requires a\n> directory, and a file would not work\", I expected this to give\n> the programmer finer controls, such as:\n> \n> - The name must refer to an existing entity on the filesystem,\n> or an existing entity on the filesystem must not exist, or\n> anything is fine (tristate).\n> \n> - The name refers to a diretory, or the name refers to a regular\n> file, or the name refers to a symbolic link, or anything goes.\n> \n> That is merely \"somewhat\", as the latter _can_ be coded (e.g. if you\n> care that the given path already exists as a directory, stat() it\n> and see if it is, or if you want that the given path does not exist\n> yet, stat() it to make sure you get ENOENT) after the option is\n> parsed by the program that uses the parser.\n> \n> But the infrastructure to allow the latter would free you from\n> having to say N_(\"file\") or N_(\"directory\"); if a parameter can\n> refer to either a file or a directory, the parse-options library\n> could give you N_(\"file or directory\") because you are already\n> telling what kind(s) of paths are allowed.\n\nSo, do you suggest to extend OPTION_FILENAME, and introduce several\nmacros using it, or a macro taking a bitfield to be filled with\nPATH_OPT_FILE, PATH_OPT_DIR, PATH_OPT_EXISTS, PATH_OPT_ABSENT,\nPATH_OPT_MASK, PATH_OPT_MODE (require (mode & MASK == MODE))?\n\nSounds like the big solution to a small problem I had with the word\n\"file\" for a dir...\n\n>> #define OPT_COLOR_FLAG(s, l, v, h) \\\n>> \t{ OPTION_CALLBACK, (s), (l), (v), N_(\"when\"), (h), PARSE_OPT_OPTARG, \\\n>> \t\tparse_opt_color_flag_cb, (intptr_t)\"always\" }\n"}]}