{"thread":{"id":"30244","subject":"[PATCH] cherry-pick: do not expect file arguments","startedAt":"2012-04-14T19:04:48Z","lastAt":"2012-04-14T23:48:56Z","messageCount":2,"participants":["Clemens Buchacher","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"189275","messageId":"20120414190448.GA26209@ecki","threadId":"30244","inReplyTo":null,"subject":"[PATCH] cherry-pick: do not expect file arguments","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2012-04-14T19:04:48Z","receivedAt":"2012-04-14T19:04:48Z","isPatch":true,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"If a commit-ish passed to cherry-pick or revert happens to have a file\nof the same name, git complains that the argument is ambiguous and\nadvises to use '--'. To make things worse, the '--' argument is removed\nby parse_options, und so passing '--' has no effect.\n\nInstead, always interpret cherry-pick/revert arguments as revisions.\n\nSigned-off-by: Clemens Buchacher <drizzd@aon.at>\n---\n builtin/revert.c |    5 ++++-\n revision.c       |   24 ++++++++++++++----------\n revision.h       |    1 +\n 3 files changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex e6840f2..92f3fa5 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -181,12 +181,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \tif (opts->subcommand != REPLAY_NONE) {\n \t\topts->revs = NULL;\n \t} else {\n+\t\tstruct setup_revision_opt s_r_opt;\n \t\topts->revs = xmalloc(sizeof(*opts->revs));\n \t\tinit_revisions(opts->revs, NULL);\n \t\topts->revs->no_walk = 1;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n-\t\targc = setup_revisions(argc, argv, opts->revs, NULL);\n+\t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n+\t\ts_r_opt.assume_dashdash = 1;\n+\t\targc = setup_revisions(argc, argv, opts->revs, &s_r_opt);\n \t}\n \n \tif (argc > 1)\ndiff --git a/revision.c b/revision.c\nindex b3554ed..9a0d9c7 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1715,17 +1715,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\tsubmodule = opt->submodule;\n \n \t/* First, search for \"--\" */\n-\tseen_dashdash = 0;\n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (strcmp(arg, \"--\"))\n-\t\t\tcontinue;\n-\t\targv[i] = NULL;\n-\t\targc = i;\n-\t\tif (argv[i + 1])\n-\t\t\tappend_prune_data(&prune_data, argv + i + 1);\n+\tif (opt && opt->assume_dashdash) {\n \t\tseen_dashdash = 1;\n-\t\tbreak;\n+\t} else {\n+\t\tseen_dashdash = 0;\n+\t\tfor (i = 1; i < argc; i++) {\n+\t\t\tconst char *arg = argv[i];\n+\t\t\tif (strcmp(arg, \"--\"))\n+\t\t\t\tcontinue;\n+\t\t\targv[i] = NULL;\n+\t\t\targc = i;\n+\t\t\tif (argv[i + 1])\n+\t\t\t\tappend_prune_data(&prune_data, argv + i + 1);\n+\t\t\tseen_dashdash = 1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \t/* Second, deal with arguments and options */\ndiff --git a/revision.h b/revision.h\nindex b8e9223..1a08384 100644\n--- a/revision.h\n+++ b/revision.h\n@@ -183,6 +183,7 @@ struct setup_revision_opt {\n \tconst char *def;\n \tvoid (*tweak)(struct rev_info *, struct setup_revision_opt *);\n \tconst char *submodule;\n+\tint assume_dashdash;\n };\n \n extern void init_revisions(struct rev_info *revs, const char *prefix);\n-- \n1.7.9.6\n"},{"id":"189306","messageId":"7vehrp27tj.fsf@alter.siamese.dyndns.org","threadId":"30244","inReplyTo":"20120414190448.GA26209@ecki","subject":"Re: [PATCH] cherry-pick: do not expect file arguments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-04-14T23:48:56Z","receivedAt":"2012-04-14T23:48:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> If a commit-ish passed to cherry-pick or revert happens to have a file\n> of the same name, git complains that the argument is ambiguous and\n> advises to use '--'. To make things worse, the '--' argument is removed\n> by parse_options, und so passing '--' has no effect.\n\nThanks.\n\nI can see how this patch is one way to solve it, but if the command knows\nthat it is feedling only revs and no pathspecs, isn't the caller the one\nthat is responsible for adding \"--\" to the argv_array it is passing to\nsetup_revisions()?\n\nWith s/assume_dashdash/no_pathspecs/, the damage to the revision traversal\nmachinery does not look _too_ bad, but I am not convinced (yet) that this\npatch is the best way to solve the issue.\n\n> Instead, always interpret cherry-pick/revert arguments as revisions.\n>\n> Signed-off-by: Clemens Buchacher <drizzd@aon.at>\n> ---\n>  builtin/revert.c |    5 ++++-\n>  revision.c       |   24 ++++++++++++++----------\n>  revision.h       |    1 +\n>  3 files changed, 19 insertions(+), 11 deletions(-)\n>\n> diff --git a/builtin/revert.c b/builtin/revert.c\n> index e6840f2..92f3fa5 100644\n> --- a/builtin/revert.c\n> +++ b/builtin/revert.c\n> @@ -181,12 +181,15 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n>  \tif (opts->subcommand != REPLAY_NONE) {\n>  \t\topts->revs = NULL;\n>  \t} else {\n> +\t\tstruct setup_revision_opt s_r_opt;\n>  \t\topts->revs = xmalloc(sizeof(*opts->revs));\n>  \t\tinit_revisions(opts->revs, NULL);\n>  \t\topts->revs->no_walk = 1;\n>  \t\tif (argc < 2)\n>  \t\t\tusage_with_options(usage_str, options);\n> -\t\targc = setup_revisions(argc, argv, opts->revs, NULL);\n> +\t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n> +\t\ts_r_opt.assume_dashdash = 1;\n> +\t\targc = setup_revisions(argc, argv, opts->revs, &s_r_opt);\n>  \t}\n>  \n>  \tif (argc > 1)\n> diff --git a/revision.c b/revision.c\n> index b3554ed..9a0d9c7 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1715,17 +1715,21 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n>  \t\tsubmodule = opt->submodule;\n>  \n>  \t/* First, search for \"--\" */\n> -\tseen_dashdash = 0;\n> -\tfor (i = 1; i < argc; i++) {\n> -\t\tconst char *arg = argv[i];\n> -\t\tif (strcmp(arg, \"--\"))\n> -\t\t\tcontinue;\n> -\t\targv[i] = NULL;\n> -\t\targc = i;\n> -\t\tif (argv[i + 1])\n> -\t\t\tappend_prune_data(&prune_data, argv + i + 1);\n> +\tif (opt && opt->assume_dashdash) {\n>  \t\tseen_dashdash = 1;\n> -\t\tbreak;\n> +\t} else {\n> +\t\tseen_dashdash = 0;\n> +\t\tfor (i = 1; i < argc; i++) {\n> +\t\t\tconst char *arg = argv[i];\n> +\t\t\tif (strcmp(arg, \"--\"))\n> +\t\t\t\tcontinue;\n> +\t\t\targv[i] = NULL;\n> +\t\t\targc = i;\n> +\t\t\tif (argv[i + 1])\n> +\t\t\t\tappend_prune_data(&prune_data, argv + i + 1);\n> +\t\t\tseen_dashdash = 1;\n> +\t\t\tbreak;\n> +\t\t}\n>  \t}\n>  \n>  \t/* Second, deal with arguments and options */\n> diff --git a/revision.h b/revision.h\n> index b8e9223..1a08384 100644\n> --- a/revision.h\n> +++ b/revision.h\n> @@ -183,6 +183,7 @@ struct setup_revision_opt {\n>  \tconst char *def;\n>  \tvoid (*tweak)(struct rev_info *, struct setup_revision_opt *);\n>  \tconst char *submodule;\n> +\tint assume_dashdash;\n>  };\n>  \n>  extern void init_revisions(struct rev_info *revs, const char *prefix);\n"}]}