{"thread":{"id":"11006","subject":"[PATCH 2/3] Move pathspec validation into interactive_add","startedAt":"2007-11-25T13:15:39Z","lastAt":"2007-11-26T04:14:38Z","messageCount":12,"participants":["Wincent Colaiuta","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"60833","messageId":"1195996542-86074-1-git-send-email-win@wincent.com","threadId":"11006","inReplyTo":null,"subject":"[PATCH 0/3] Updates to git-add--interactive","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T13:15:39Z","receivedAt":"2007-11-25T13:15:39Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Here's an updated series, this time hopefully with the braindead\nparts removed.\n\n[PATCH 1/3] Add \"--patch\" option to git-add--interactive\n\n    Unchanged.\n\n[PATCH 2/3] Move pathspec validation into interactive_add\n\n    Braindeadness removed.\n\n[PATCH 3/3] Rename patch_update_file function to patch_update_pathspec\n\n    Only modified insofar as to make it apply on top of PATCH 2.\n\nThe one remaining issue is that pathspecs like \"\\*.h\" don't work.\nBut from comments earlier on in this thread I had the impression\nthat they should.\n\nThey don't work because such pathspecs don't work for the underlying\ngit-diff-files implementation that's used by git-add--interactive:\n\ngit diff-files --numstat --summary -- \\*.h\n\nIs some alternative being proposed that would allow them to work?\n"},{"id":"60832","messageId":"1195996542-86074-2-git-send-email-win@wincent.com","threadId":"11006","inReplyTo":"1195996542-86074-1-git-send-email-win@wincent.com","subject":"[PATCH 1/3] Rename patch_update_file function to patch_update_pathspec","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T13:15:40Z","receivedAt":"2007-11-25T13:15:40Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"The patch_update_file function really works on pathspecs, not files, so\nrename it to reflect its actual purpose.\n\nSigned-off-by: Wincent Colaiuta <win@wincent.com>\n---\n git-add--interactive.perl |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex e347216..15b2c9f 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -571,11 +571,11 @@ sub patch_update_cmd {\n \t\t\t\t       HEADER => $status_head, },\n \t\t\t\t     @mods);\n \tfor (@them) {\n-\t\tpatch_update_file($_->{VALUE});\n+\t\tpatch_update_pathspec($_->{VALUE});\n \t}\n }\n \n-sub patch_update_file {\n+sub patch_update_pathspec {\n \tmy ($ix, $num);\n \tmy $path = shift;\n \tmy ($head, @hunk) = parse_diff($path);\n-- \n1.5.3.6.1994.g38001\n"},{"id":"60831","messageId":"1195996542-86074-3-git-send-email-win@wincent.com","threadId":"11006","inReplyTo":"1195996542-86074-2-git-send-email-win@wincent.com","subject":"[PATCH 2/3] Move pathspec validation into interactive_add","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T13:15:41Z","receivedAt":"2007-11-25T13:15:41Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Simplify git-add--interactive by moving the pathspec validation into the\ninteractive_add() function of builtin-add. We can do this because\nbuiltin-add is the only caller of git-add--interactive.\n\nThe validate_pathspec() function added by this commit is based on a\nsample posted to the mailing list by Junio Hamano.\n\nSigned-off-by: Wincent Colaiuta <win@wincent.com>\n---\n builtin-add.c             |   33 +++++++++++++++++++++++++++++----\n builtin-commit.c          |    2 +-\n commit.h                  |    2 +-\n git-add--interactive.perl |   12 ++----------\n 4 files changed, 33 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex dd895df..870f4a1 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -135,12 +135,37 @@ static void refresh(int verbose, const char **pathspec)\n         free(seen);\n }\n \n-int interactive_add(int argc, const char **argv)\n+static int validate_pathspec(const char *prefix, const char **patterns)\n+{\n+\tint i, ret = 0;\n+\tchar *m;\n+\tif (!patterns || !*patterns)\n+\t\treturn 0;\n+\tif (read_cache() < 0)\n+\t\tdie(\"index file corrupt\");\n+\t(void)get_pathspec(prefix, patterns);\n+\tfor (i = 0; patterns[i]; i++)\n+\t\t;\n+\tm = xcalloc(1, i);\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\t(void)pathspec_match(patterns, m, ce->name, 0);\n+\t}\n+\tret = report_path_error(m, patterns, prefix ? strlen(prefix) : 0);\n+\tfree(m);\n+\treturn ret;\n+}\n+\n+int interactive_add(const char *prefix, int argc, const char **argv)\n {\n \tint status;\n-\tconst char **args = xmalloc(sizeof(const char *) * (argc + 1));\n+\tconst char **args;\n+\tif ((status = validate_pathspec(prefix, argv)))\n+\t\treturn status;\n+\targs = xmalloc(sizeof(const char *) * (argc + 2));\n \targs[0] = \"add--interactive\";\n-\tmemcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * argc);\n+\tmemcpy((void *)args + sizeof(const char *),\n+\t    argv, sizeof(const char *) * argc);\n \targs[argc + 1] = NULL;\n \n \tstatus = run_command_v_opt(args, RUN_GIT_CMD);\n@@ -176,7 +201,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n \tif (add_interactive)\n-\t\texit(interactive_add(argc, argv));\n+\t\texit(interactive_add(prefix, argc, argv));\n \n \tgit_config(git_default_config);\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 5d27102..95d1c0d 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -165,7 +165,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)\n \tconst char **pathspec = NULL;\n \n \tif (interactive) {\n-\t\tinteractive_add(argc, argv);\n+\t\tinteractive_add(prefix, argc, argv);\n \t\tcommit_style = COMMIT_AS_IS;\n \t\treturn get_index_file();\n \t}\ndiff --git a/commit.h b/commit.h\nindex 9f0765b..dc6fe31 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -113,7 +113,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,\n \n int in_merge_bases(struct commit *, struct commit **, int);\n \n-extern int interactive_add(int argc, const char **argv);\n+extern int interactive_add(const char *prefix, int argc, const char **argv);\n extern int rerere(void);\n \n static inline int single_parent(struct commit *commit)\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 15b2c9f..381bcbe 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -56,17 +56,9 @@ sub list_modified {\n \tmy ($only) = @_;\n \tmy (%data, @return);\n \tmy ($add, $del, $adddel, $file);\n-\tmy @tracked = ();\n-\n-\tif (@ARGV) {\n-\t\t@tracked = map {\n-\t\t\tchomp $_; $_;\n-\t\t} run_cmd_pipe(qw(git ls-files --exclude-standard --), @ARGV);\n-\t\treturn if (!@tracked);\n-\t}\n \n \tfor (run_cmd_pipe(qw(git diff-index --cached\n-\t\t\t     --numstat --summary HEAD --), @tracked)) {\n+\t\t\t     --numstat --summary HEAD --), @ARGV)) {\n \t\tif (($add, $del, $file) =\n \t\t    /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n \t\t\tmy ($change, $bin);\n@@ -89,7 +81,7 @@ sub list_modified {\n \t\t}\n \t}\n \n-\tfor (run_cmd_pipe(qw(git diff-files --numstat --summary --), @tracked)) {\n+\tfor (run_cmd_pipe(qw(git diff-files --numstat --summary --), @ARGV)) {\n \t\tif (($add, $del, $file) =\n \t\t    /^([-\\d]+)\t([-\\d]+)\t(.*)/) {\n \t\t\tif (!exists $data{$file}) {\n-- \n1.5.3.6.1994.g38001\n"},{"id":"60834","messageId":"1195996542-86074-4-git-send-email-win@wincent.com","threadId":"11006","inReplyTo":"1195996542-86074-3-git-send-email-win@wincent.com","subject":"[PATCH 3/3] Add \"--patch\" option to git-add--interactive","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T13:15:42Z","receivedAt":"2007-11-25T13:15:42Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"When the \"--patch\" option is supplied, the patch_update_file function is\ncalled, once for each supplied pathspec argument, and then we exit.\n\nSeeing as builtin-add is the only caller of git-add--interactive we can\nimpose a strict requirement on the format of the arguments to avoid\npossible ambiguity: an \"--\" argument must be used whenever any pathspecs\nare passed, both with the \"--patch\" option and without it.\n\nThis commit adds an early return mechanism to the patch_update_pathspec\nfunction to prevent spurious line feeds from being echoed when the user\npasses in pathspecs which match unchanged files.\n\nSigned-off-by: Wincent Colaiuta <win@wincent.com>\n---\n Documentation/git-add.txt |    9 ++++++++-\n builtin-add.c             |   17 ++++++++++-------\n git-add--interactive.perl |   35 ++++++++++++++++++++++++++++++++---\n 3 files changed, 50 insertions(+), 11 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 63829d9..ce22de8 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -61,7 +61,14 @@ OPTIONS\n \n -i, \\--interactive::\n \tAdd modified contents in the working tree interactively to\n-\tthe index.\n+\tthe index. Optional path arguments may be supplied to limit\n+\toperation to a subset of the working tree. See ``Interactive\n+\tmode'' for details.\n+\n+-p, \\--patch:\n+\tSimilar to Interactive mode but the initial command loop is\n+\tbypassed and the 'patch' subcommand is invoked using each of\n+\tthe specified filepatterns before exiting.\n \n -u::\n \tUpdate only files that git already knows about. This is similar\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 870f4a1..2172e7e 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -19,7 +19,7 @@ static const char * const builtin_add_usage[] = {\n \t\"git-add [options] [--] <filepattern>...\",\n \tNULL\n };\n-\n+static int patch_interactive = 0, add_interactive = 0;\n static int take_worktree_changes;\n \n static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n@@ -158,15 +158,18 @@ static int validate_pathspec(const char *prefix, const char **patterns)\n \n int interactive_add(const char *prefix, int argc, const char **argv)\n {\n-\tint status;\n+\tint status, pre_argc;\n \tconst char **args;\n \tif ((status = validate_pathspec(prefix, argv)))\n \t\treturn status;\n-\targs = xmalloc(sizeof(const char *) * (argc + 2));\n+\tpre_argc = patch_interactive ? 3 : 2;\n+\targs = xmalloc(sizeof(const char *) * (argc + pre_argc + 1));\n \targs[0] = \"add--interactive\";\n-\tmemcpy((void *)args + sizeof(const char *),\n+\targs[1]\t= patch_interactive ? \"--patch\" : \"--\";\n+\targs[2] = \"--\";\n+\tmemcpy((void *)args + sizeof(const char *) * pre_argc,\n \t    argv, sizeof(const char *) * argc);\n-\targs[argc + 1] = NULL;\n+\targs[argc + pre_argc] = NULL;\n \n \tstatus = run_command_v_opt(args, RUN_GIT_CMD);\n \tfree(args);\n@@ -179,13 +182,13 @@ static const char ignore_error[] =\n \"The following paths are ignored by one of your .gitignore files:\\n\";\n \n static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n-static int add_interactive = 0;\n \n static struct option builtin_add_options[] = {\n \tOPT__DRY_RUN(&show_only),\n \tOPT__VERBOSE(&verbose),\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n+\tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n \tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n@@ -200,7 +203,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n-\tif (add_interactive)\n+\tif (add_interactive || patch_interactive)\n \t\texit(interactive_add(prefix, argc, argv));\n \n \tgit_config(git_default_config);\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 381bcbe..7d62ceb 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,9 @@\n \n use strict;\n \n+# command line options\n+my $patch;\n+\n sub run_cmd_pipe {\n \tif ($^O eq 'MSWin32') {\n \t\tmy @invalid = grep {m/[\":*]/} @_;\n@@ -335,7 +338,8 @@ sub add_untracked_cmd {\n \n sub parse_diff {\n \tmy ($path) = @_;\n-\tmy @diff = run_cmd_pipe(qw(git diff-files -p --), $path);\n+\tmy @diff = run_cmd_pipe(qw(git diff-files -p --), $path)\n+\t    or return undef;\n \tmy (@hunk) = { TEXT => [] };\n \n \tfor (@diff) {\n@@ -571,6 +575,7 @@ sub patch_update_pathspec {\n \tmy ($ix, $num);\n \tmy $path = shift;\n \tmy ($head, @hunk) = parse_diff($path);\n+\treturn unless $head;\n \tfor (@{$head->{TEXT}}) {\n \t\tprint;\n \t}\n@@ -775,6 +780,20 @@ add untracked - add contents of untracked files to the staged set of changes\n EOF\n }\n \n+sub process_args {\n+\treturn unless @ARGV;\n+\tmy $arg = shift @ARGV;\n+\tif ($arg eq \"--patch\") {\n+\t\t$patch = 1;\n+\t\t$arg = shift @ARGV or die \"missing --\";\n+\t\tdie \"invalid argument $arg, expecting --\"\n+\t\t    unless $arg eq \"--\";\n+\t}\n+\telsif ($arg ne \"--\") {\n+\t\tdie \"invalid argument $arg, expecting --\";\n+\t}\n+}\n+\n sub main_loop {\n \tmy @cmd = ([ 'status', \\&status_cmd, ],\n \t\t   [ 'update', \\&update_cmd, ],\n@@ -803,6 +822,16 @@ sub main_loop {\n \t}\n }\n \n+process_args();\n refresh();\n-status_cmd();\n-main_loop();\n+if ($patch) {\n+\tprint \"No filepattern specified: what did you want to patch?\\n\"\n+\t    unless @ARGV;\n+\tforeach my $pathspec (@ARGV) {\n+\t\tpatch_update_pathspec($pathspec);\n+\t}\n+}\n+else {\n+\tstatus_cmd();\n+\tmain_loop();\n+}\n-- \n1.5.3.6.1994.g38001\n"},{"id":"60839","messageId":"7vd4tynqpw.fsf@gitster.siamese.dyndns.org","threadId":"11006","inReplyTo":"1195996542-86074-1-git-send-email-win@wincent.com","subject":"[PATCH] builtin-add: fix command line building to call interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T18:07:55Z","receivedAt":"2007-11-25T18:07:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The earlier 7c0ab4458994aa895855abc4a504cf693ecc0cf1 (Teach builtin-add\nto pass multiple paths to git-add--interactive) did not allocate enough,\nand had unneeded (void*) pointer arithmetic.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n builtin-add.c |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex dd895df..7c6a296 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -138,9 +138,10 @@ static void refresh(int verbose, const char **pathspec)\n int interactive_add(int argc, const char **argv)\n {\n \tint status;\n-\tconst char **args = xmalloc(sizeof(const char *) * (argc + 1));\n+\tconst char **args = xcalloc(sizeof(const char *), (argc + 2));\n+\n \targs[0] = \"add--interactive\";\n-\tmemcpy((void *)args + sizeof(const char *), argv, sizeof(const char *) * argc);\n+\tmemcpy(&(args[1]), argv, sizeof(const char *) * argc);\n \targs[argc + 1] = NULL;\n \n \tstatus = run_command_v_opt(args, RUN_GIT_CMD);\n-- \n1.5.3.6.2014.g7500f\n"},{"id":"60840","messageId":"7v7ik6nqm5.fsf@gitster.siamese.dyndns.org","threadId":"11006","inReplyTo":"1195996542-86074-1-git-send-email-win@wincent.com","subject":"[PATCH] add -i: Fix running from a subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T18:10:10Z","receivedAt":"2007-11-25T18:10:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This fixes the pathspec interactive_add() passes to the underlying\ngit-add--interactive helper.  When the command was run from a\nsubdirectory, cmd_add() already has gone up to the toplevel of the work\ntree, and the helper will be spawned from there.  The pathspec given on\nthe command line from the user needs to be adjusted for this.\n\nThis adds \"validate_pathspec()\" function in the callchain, but it does\nnot validate yet.  The function can be changed to barf if there are\nunmatching pathspec given by the user, but that is not strictly\nnecessary.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * The reason it is not \"strictly necessary\" is because if you give\n   unmatching pathspecs, you simply would not see their effects (good or\n   bad) in any of the subcommands in git-add--interactive.\n\n builtin-add.c    |   24 ++++++++++++++++++++----\n builtin-commit.c |    2 +-\n commit.h         |    2 +-\n 3 files changed, 22 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 7c6a296..865c475 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -135,13 +135,29 @@ static void refresh(int verbose, const char **pathspec)\n         free(seen);\n }\n \n-int interactive_add(int argc, const char **argv)\n+static const char **validate_pathspec(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char **pathspec = get_pathspec(prefix, argv);\n+\n+\treturn pathspec;\n+}\n+\n+int interactive_add(int argc, const char **argv, const char *prefix)\n {\n \tint status;\n-\tconst char **args = xcalloc(sizeof(const char *), (argc + 2));\n+\tconst char **args;\n+\tconst char **pathspec = NULL;\n+\n+\tif (argc) {\n+\t\tpathspec = validate_pathspec(argc, argv, prefix);\n+\t\tif (!pathspec)\n+\t\t\treturn -1;\n+\t}\n \n+\targs = xcalloc(sizeof(const char *), (argc + 2));\n \targs[0] = \"add--interactive\";\n-\tmemcpy(&(args[1]), argv, sizeof(const char *) * argc);\n+\tif (argc)\n+\t\tmemcpy(&(args[1]), pathspec, sizeof(const char *) * argc);\n \targs[argc + 1] = NULL;\n \n \tstatus = run_command_v_opt(args, RUN_GIT_CMD);\n@@ -177,7 +193,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n \tif (add_interactive)\n-\t\texit(interactive_add(argc, argv));\n+\t\texit(interactive_add(argc, argv, prefix));\n \n \tgit_config(git_default_config);\n \ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 5d27102..45e51b1 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -165,7 +165,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix)\n \tconst char **pathspec = NULL;\n \n \tif (interactive) {\n-\t\tinteractive_add(argc, argv);\n+\t\tinteractive_add(argc, argv, prefix);\n \t\tcommit_style = COMMIT_AS_IS;\n \t\treturn get_index_file();\n \t}\ndiff --git a/commit.h b/commit.h\nindex 9f0765b..10e2b5d 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -113,7 +113,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,\n \n int in_merge_bases(struct commit *, struct commit **, int);\n \n-extern int interactive_add(int argc, const char **argv);\n+extern int interactive_add(int argc, const char **argv, const char *prefix);\n extern int rerere(void);\n \n static inline int single_parent(struct commit *commit)\n-- \n1.5.3.6.2014.g7500f\n"},{"id":"60841","messageId":"7vzlx2mc02.fsf@gitster.siamese.dyndns.org","threadId":"11006","inReplyTo":"1195996542-86074-2-git-send-email-win@wincent.com","subject":"Re: [PATCH 1/3] Rename patch_update_file function to patch_update_pathspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T18:11:09Z","receivedAt":"2007-11-25T18:11:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wincent Colaiuta <win@wincent.com> writes:\n\n> The patch_update_file function really works on pathspecs, not files, so\n> rename it to reflect its actual purpose.\n\nI do not think this is a correct change.  patch_update_file() should\nwork on files not pathspecs.\n\nA list of exact pathnames are valid pathspecs, and because the matching\nfunction takes exact match first, if you feed a command\n(e.g. diff-files) paths expanded from the pathspec given by the user,\nyou should be able to get desired results, no?\n\npatch_update_cmd() grabs list of modified _files_, not pathspecs.  Then\nthe user chooses the ones to select hunks from via list_and_choose(),\nand you may want to pretend that everything was chosen if you are\noperating with pathspec and with --patch option.  Iterating over them is\niterating over files, not pathspecs, at that point.\n\nHave you tried the one that is in 'next'?  I think it does the right\nthing except \"the bogus pathspec validation at the beginning\" part as\nfar as the pathspec handling is concerned.\n\nI sent out two fixes on top of the series.\n"},{"id":"60842","messageId":"F09249EB-475A-4352-A3A1-A8B15D2A94FF@wincent.com","threadId":"11006","inReplyTo":"7vd4tynqpw.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-add: fix command line building to call interactive","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T18:27:24Z","receivedAt":"2007-11-25T18:27:24Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 25/11/2007, a las 19:07, Junio C Hamano escribió:\n\n> The earlier 7c0ab4458994aa895855abc4a504cf693ecc0cf1 (Teach builtin- \n> add\n> to pass multiple paths to git-add--interactive) did not allocate  \n> enough,\n\nYes, it was off by one; sorry about that. You may have noticed that I  \nfixed that up in the patches I sent out yesterday and today. May need  \nto redo them now to apply on top of this.\n\nCheers,\nWincent\n"},{"id":"60844","messageId":"7vlk8mmatu.fsf@gitster.siamese.dyndns.org","threadId":"11006","inReplyTo":"F09249EB-475A-4352-A3A1-A8B15D2A94FF@wincent.com","subject":"Re: [PATCH] builtin-add: fix command line building to call interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T18:36:29Z","receivedAt":"2007-11-25T18:36:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wincent Colaiuta <win@wincent.com> writes:\n\n> El 25/11/2007, a las 19:07, Junio C Hamano escribi󺊊> The earlier 7c0ab4458994aa895855abc4a504cf693ecc0cf1 (Teach builtin- \n>> add\n>> to pass multiple paths to git-add--interactive) did not allocate  \n>> enough,\n>\n> Yes, it was off by one; sorry about that. You may have noticed that I  \n> fixed that up in the patches I sent out yesterday and today. May need  \n> to redo them now to apply on top of this.\n\nI'd suggest you to slow down, apply the two patches on top of 'next' and\ntake a look at the result.\n\nI _think_ the only remaining thing is --patch, and none of the pathspec\nthing is needed.\n"},{"id":"60845","messageId":"22CBC161-57C2-4A7E-9415-CE8117758A44@wincent.com","threadId":"11006","inReplyTo":"7vlk8mmatu.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-add: fix command line building to call interactive","fromName":"Wincent Colaiuta","fromEmail":"win@wincent.com","sentAt":"2007-11-25T19:02:33Z","receivedAt":"2007-11-25T19:02:33Z","isPatch":true,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"El 25/11/2007, a las 19:36, Junio C Hamano escribió:\n\n> I _think_ the only remaining thing is --patch, and none of the  \n> pathspec\n> thing is needed.\n\nYou're probably right; the pathspec validation is probably not  \nnecessary (and you may recall that my original patch didn't include  \nit; I only tried adding after you said it might be appropriate to have  \nthe \"--error-unmatch\" behaviour). This is probably more convenient for  \nthe user, as it allows them to pass \"sloppy\" parameters like the  \nfollowing:\n\n\tgit-add -i *.h\n\n(Note that's \"*.h\" and not \"\\*.h\"). In the Git repository, without  \nvalidation, this just works. With strict validation, it would complain:\n\n\terror: pathspec 'common-cmds.h' did not match any file(s) known to git.\n\nSo just forgetting about the validation is probably the right thing to  \ndo.\n\nAs for adding the --patch option, I'll stand back and see if someone  \nmore skilled than I wants to do it; should only be a few lines and  \nwill save traffic to the list because they'll probably get it right  \nfirst time.\n\nCheers,\nWincent\n"},{"id":"60846","messageId":"7vprxyksxf.fsf@gitster.siamese.dyndns.org","threadId":"11006","inReplyTo":"22CBC161-57C2-4A7E-9415-CE8117758A44@wincent.com","subject":"Re: [PATCH] builtin-add: fix command line building to call interactive","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-25T19:48:28Z","receivedAt":"2007-11-25T19:48:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Wincent Colaiuta <win@wincent.com> writes:\n\n>>  I _think_ the only remaining thing is --patch, and none of the  \n>> pathspec\n>> thing is needed.\n>\n> You're probably right; the pathspec validation is probably not  \n> necessary (and you may recall that my original patch didn't include  \n> it; I only tried adding after you said it might be appropriate to have  \n> the \"--error-unmatch\" behaviour)...\n\nOh, I did not mean the validation bits I left out in the second \"fixup\"\npatch I sent earlier by that comment.  What I meant was the changes you\nhad in git-add--interactive::patch_update_cmd() to make it take non\nfiles but pathspecs.\n\n> This is probably more convenient for  \n> the user, as it allows them to pass \"sloppy\" parameters like the  \n> following:\n>\n> \tgit-add -i *.h\n>\n> (Note that's \"*.h\" and not \"\\*.h\"). In the Git repository, without  \n> validation, this just works. With strict validation, it would complain...\n\nI'd mostly agree, but we need to realize that this is a two edged sword.\nPathspecs can be leading-directories or fileglobs.  For fileglobs, you\nare right.  The user can let the shell do the globbing.  Not validating,\nhowever, also means that\n\n\tgit add -p Documentatoin\n\nwould report \"there is nothing to patch\" without being helpful, pointing\nout that the name of the directory is misspelled.\n\n> As for adding the --patch option, I'll stand back and see if someone  \n> more skilled than I wants to do it; should only be a few lines and  \n> will save traffic to the list because they'll probably get it right  \n> first time.\n\nAnd that skilled person turns out to be you ;-).\n\nAt the end of this message is your 3/3 from yesterday, minimally\nadjusted to the current tip of 'next'.\n\nI tweaked it to allow:\n\n\tgit add -p\n\nwithout any pathspecs, which I think is a valid usage.\n\n-- >8 --\n[PATCH] Add \"--patch\" option to git-add--interactive\n\nWhen the \"--patch\" option is supplied, the patch_update_cmd() function is\ncalled bypassing the main_loop() and exits.\n\nSeeing as builtin-add is the only caller of git-add--interactive we can\nimpose a strict requirement on the format of the arguments to avoid\npossible ambiguity: an \"--\" argument must be used whenever any pathspecs\nare passed, both with the \"--patch\" option and without it.\n\nSigned-off-by: Wincent Colaiuta <win@wincent.com>\n---\n Documentation/git-add.txt |    9 +++++++-\n builtin-add.c             |   24 ++++++++++++++-------\n git-add--interactive.perl |   51 ++++++++++++++++++++++++++++++++++++--------\n 3 files changed, 65 insertions(+), 19 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 63829d9..ce22de8 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -61,7 +61,14 @@ OPTIONS\n \n -i, \\--interactive::\n \tAdd modified contents in the working tree interactively to\n-\tthe index.\n+\tthe index. Optional path arguments may be supplied to limit\n+\toperation to a subset of the working tree. See ``Interactive\n+\tmode'' for details.\n+\n+-p, \\--patch:\n+\tSimilar to Interactive mode but the initial command loop is\n+\tbypassed and the 'patch' subcommand is invoked using each of\n+\tthe specified filepatterns before exiting.\n \n -u::\n \tUpdate only files that git already knows about. This is similar\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 865c475..5c29cc2 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -19,7 +19,7 @@ static const char * const builtin_add_usage[] = {\n \t\"git-add [options] [--] <filepattern>...\",\n \tNULL\n };\n-\n+static int patch_interactive = 0, add_interactive = 0;\n static int take_worktree_changes;\n \n static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n@@ -144,7 +144,7 @@ static const char **validate_pathspec(int argc, const char **argv, const char *p\n \n int interactive_add(int argc, const char **argv, const char *prefix)\n {\n-\tint status;\n+\tint status, ac;\n \tconst char **args;\n \tconst char **pathspec = NULL;\n \n@@ -154,11 +154,17 @@ int interactive_add(int argc, const char **argv, const char *prefix)\n \t\t\treturn -1;\n \t}\n \n-\targs = xcalloc(sizeof(const char *), (argc + 2));\n-\targs[0] = \"add--interactive\";\n-\tif (argc)\n-\t\tmemcpy(&(args[1]), pathspec, sizeof(const char *) * argc);\n-\targs[argc + 1] = NULL;\n+\targs = xcalloc(sizeof(const char *), (argc + 4));\n+\tac = 0;\n+\targs[ac++] = \"add--interactive\";\n+\tif (patch_interactive)\n+\t\targs[ac++] = \"--patch\";\n+\targs[ac++] = \"--\";\n+\tif (argc) {\n+\t\tmemcpy(&(args[ac]), pathspec, sizeof(const char *) * argc);\n+\t\tac += argc;\n+\t}\n+\targs[ac] = NULL;\n \n \tstatus = run_command_v_opt(args, RUN_GIT_CMD);\n \tfree(args);\n@@ -171,13 +177,13 @@ static const char ignore_error[] =\n \"The following paths are ignored by one of your .gitignore files:\\n\";\n \n static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n-static int add_interactive = 0;\n \n static struct option builtin_add_options[] = {\n \tOPT__DRY_RUN(&show_only),\n \tOPT__VERBOSE(&verbose),\n \tOPT_GROUP(\"\"),\n \tOPT_BOOLEAN('i', \"interactive\", &add_interactive, \"interactive picking\"),\n+\tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactive patching\"),\n \tOPT_BOOLEAN('f', NULL, &ignored_too, \"allow adding otherwise ignored files\"),\n \tOPT_BOOLEAN('u', NULL, &take_worktree_changes, \"update tracked files\"),\n \tOPT_BOOLEAN( 0 , \"refresh\", &refresh_only, \"don't add, only refresh the index\"),\n@@ -192,6 +198,8 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \targc = parse_options(argc, argv, builtin_add_options,\n \t\t\t  builtin_add_usage, 0);\n+\tif (patch_interactive)\n+\t\tadd_interactive = 1;\n \tif (add_interactive)\n \t\texit(interactive_add(argc, argv, prefix));\n \ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex e347216..df5df3e 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -2,6 +2,9 @@\n \n use strict;\n \n+# command line options\n+my $patch_mode;\n+\n sub run_cmd_pipe {\n \tif ($^O eq 'MSWin32') {\n \t\tmy @invalid = grep {m/[\":*]/} @_;\n@@ -552,8 +555,8 @@ sub help_patch_cmd {\n \tprint <<\\EOF ;\n y - stage this hunk\n n - do not stage this hunk\n-a - stage this and all the remaining hunks\n-d - do not stage this hunk nor any of the remaining hunks\n+a - stage this and all the remaining hunks in the file\n+d - do not stage this hunk nor any of the remaining hunks in the file\n j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n@@ -563,13 +566,21 @@ EOF\n }\n \n sub patch_update_cmd {\n-\tmy @mods = list_modified('file-only');\n-\t@mods = grep { !($_->{BINARY}) } @mods;\n-\treturn if (!@mods);\n+\tmy @mods = grep { !($_->{BINARY}) } list_modified('file-only');\n+\tmy @them;\n \n-\tmy (@them) = list_and_choose({ PROMPT => 'Patch update',\n-\t\t\t\t       HEADER => $status_head, },\n-\t\t\t\t     @mods);\n+\tif (!@mods) {\n+\t\tprint STDERR \"No changes.\\n\";\n+\t\treturn 0;\n+\t}\n+\tif ($patch_mode) {\n+\t\t@them = @mods;\n+\t}\n+\telse {\n+\t\t@them = list_and_choose({ PROMPT => 'Patch update',\n+\t\t\t\t\t  HEADER => $status_head, },\n+\t\t\t\t\t@mods);\n+\t}\n \tfor (@them) {\n \t\tpatch_update_file($_->{VALUE});\n \t}\n@@ -783,6 +794,20 @@ add untracked - add contents of untracked files to the staged set of changes\n EOF\n }\n \n+sub process_args {\n+\treturn unless @ARGV;\n+\tmy $arg = shift @ARGV;\n+\tif ($arg eq \"--patch\") {\n+\t\t$patch_mode = 1;\n+\t\t$arg = shift @ARGV or die \"missing --\";\n+\t\tdie \"invalid argument $arg, expecting --\"\n+\t\t    unless $arg eq \"--\";\n+\t}\n+\telsif ($arg ne \"--\") {\n+\t\tdie \"invalid argument $arg, expecting --\";\n+\t}\n+}\n+\n sub main_loop {\n \tmy @cmd = ([ 'status', \\&status_cmd, ],\n \t\t   [ 'update', \\&update_cmd, ],\n@@ -811,6 +836,12 @@ sub main_loop {\n \t}\n }\n \n+process_args();\n refresh();\n-status_cmd();\n-main_loop();\n+if ($patch_mode) {\n+\tpatch_update_cmd();\n+}\n+else {\n+\tstatus_cmd();\n+\tmain_loop();\n+}\n-- \n1.5.3.6.2018.g84394\n"},{"id":"60902","messageId":"20071126041437.GA6471@sigill.intra.peff.net","threadId":"11006","inReplyTo":"7vprxyksxf.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] builtin-add: fix command line building to call interactive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-11-26T04:14:38Z","receivedAt":"2007-11-26T04:14:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Nov 25, 2007 at 11:48:28AM -0800, Junio C Hamano wrote:\n\n> > \tgit-add -i *.h\n> >\n> > (Note that's \"*.h\" and not \"\\*.h\"). In the Git repository, without  \n> > validation, this just works. With strict validation, it would complain...\n> \n> I'd mostly agree, but we need to realize that this is a two edged sword.\n> Pathspecs can be leading-directories or fileglobs.  For fileglobs, you\n> are right.  The user can let the shell do the globbing.  Not validating,\n> however, also means that\n> \n> \tgit add -p Documentatoin\n> \n> would report \"there is nothing to patch\" without being helpful, pointing\n> out that the name of the directory is misspelled.\n\nI think the problem there is not validation, but that the previous\nproposal was validating the wrong thing. IOW, the user doesn't want a\ncomplaint \"this file is not tracked by git\" (which catches untracked\nthings with *.h) but rather \"this file does not even exist\" (which\ncatches typos like Documentatoin).\n\nSo it is not really a git pathspec being provided (from the user's point\nof view), but rather something else (a pathspec _or_ a working tree\nfile).\n\n-Peff\n"}]}