{"thread":{"id":"8100","subject":"[PATCH] git-add: allow path limiting with -u","startedAt":"2007-05-12T06:42:00Z","lastAt":"2007-05-14T01:29:31Z","messageCount":9,"participants":["Jeff King","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"41895","messageId":"20070512064159.GA7471@coredump.intra.peff.net","threadId":"8100","inReplyTo":null,"subject":"[PATCH] git-add: allow path limiting with -u","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-05-12T06:42:00Z","receivedAt":"2007-05-12T06:42:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Rather than updating all working tree paths, we limit\nourselves to paths listed on the command line.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThis turned out to be quite easy to implement. Patch is slightly larger\nthan necessary due to removing _all_ from the variable names, but I\nthink that better expresses the new functionality.\n\nI'm not sure that the documentation needs updated at all; I had just\nassumed after reading it that 'git-add -u foo' would DWIM.\n\n builtin-add.c         |   13 ++++++-------\n t/t2200-add-update.sh |   38 ++++++++++++++++++++++++++++++++++++++\n 2 files changed, 44 insertions(+), 7 deletions(-)\n create mode 100755 t/t2200-add-update.sh\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 5e6748f..1591171 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -16,7 +16,7 @@\n static const char builtin_add_usage[] =\n \"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--] <filepattern>...\";\n \n-static int take_all_worktree_changes;\n+static int take_worktree_changes;\n static const char *excludes_file;\n \n static void prune_directory(struct dir_struct *dir, const char **pathspec, int prefix)\n@@ -122,11 +122,12 @@ static void update_callback(struct diff_queue_struct *q,\n \t}\n }\n \n-static void update_all(int verbose)\n+static void update(int verbose, const char **files)\n {\n \tstruct rev_info rev;\n \tinit_revisions(&rev, \"\");\n \tsetup_revisions(0, NULL, &rev, NULL);\n+\trev.prune_data = get_pathspec(rev.prefix, files);\n \trev.diffopt.output_format = DIFF_FORMAT_CALLBACK;\n \trev.diffopt.format_callback = update_callback;\n \trev.diffopt.format_callback_data = &verbose;\n@@ -200,16 +201,14 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"-u\")) {\n-\t\t\ttake_all_worktree_changes = 1;\n+\t\t\ttake_worktree_changes = 1;\n \t\t\tcontinue;\n \t\t}\n \t\tusage(builtin_add_usage);\n \t}\n \n-\tif (take_all_worktree_changes) {\n-\t\tif (i < argc)\n-\t\t\tdie(\"-u and explicit paths are incompatible\");\n-\t\tupdate_all(verbose);\n+\tif (take_worktree_changes) {\n+\t\tupdate(verbose, argv + i);\n \t\tgoto finish;\n \t}\n \ndiff --git a/t/t2200-add-update.sh b/t/t2200-add-update.sh\nnew file mode 100755\nindex 0000000..83005e7\n--- /dev/null\n+++ b/t/t2200-add-update.sh\n@@ -0,0 +1,38 @@\n+#!/bin/sh\n+\n+test_description='git-add -u with path limiting\n+\n+This test creates a working tree state with three files:\n+\n+  top (previously committed, modified)\n+  dir/sub (previously committed, modified)\n+  dir/other (untracked)\n+\n+and issues a git-add -u with path limiting on \"dir\" to add\n+only the updates to dir/sub.'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'setup' '\n+echo initial >top &&\n+mkdir dir &&\n+echo initial >dir/sub &&\n+git-add dir/sub top &&\n+git-commit -m initial &&\n+echo changed >top &&\n+echo changed >dir/sub &&\n+echo other >dir/other\n+'\n+\n+test_expect_success 'update' 'git-add -u dir'\n+\n+test_expect_success 'update touched correct path' \\\n+  'test \"`git-diff-files --name-status dir/sub`\" = \"\"'\n+\n+test_expect_success 'update did not touch other tracked files' \\\n+  'test \"`git-diff-files --name-status top`\" = \"M\ttop\"'\n+\n+test_expect_success 'update did not touch untracked files' \\\n+  'test \"`git-diff-files --name-status dir/other`\" = \"\"'\n+\n+test_done\n-- \n1.5.2.rc3.709.g07945-dirty\n"},{"id":"41898","messageId":"7vejlm5xax.fsf@assigned-by-dhcp.cox.net","threadId":"8100","inReplyTo":"20070512064159.GA7471@coredump.intra.peff.net","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-12T07:13:42Z","receivedAt":"2007-05-12T07:13:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Rather than updating all working tree paths, we limit\n> ourselves to paths listed on the command line.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This turned out to be quite easy to implement. Patch is slightly larger\n> than necessary due to removing _all_ from the variable names, but I\n> think that better expresses the new functionality.\n>\n> I'm not sure that the documentation needs updated at all; I had just\n> assumed after reading it that 'git-add -u foo' would DWIM.\n\nWonderful, and I agree it should be almost trivial once you\nunderstand how diff works.  Will take a look.\n"},{"id":"42064","messageId":"200705131235.25281.jnareb@gmail.com","threadId":"8100","inReplyTo":"20070512064159.GA7471@coredump.intra.peff.net","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-05-13T10:35:24Z","receivedAt":"2007-05-13T10:35:24Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sat, 12 May 2007, Jeff King wrote:\n\n> Rather than updating all working tree paths, we limit\n> ourselves to paths listed on the command line.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> This turned out to be quite easy to implement. Patch is slightly larger\n> than necessary due to removing _all_ from the variable names, but I\n> think that better expresses the new functionality.\n> \n> I'm not sure that the documentation needs updated at all; I had just\n> assumed after reading it that 'git-add -u foo' would DWIM.\n\nDo git-add *needs* path specifier (even if it is '.') also for `-u'?\nThe changes in documentation were to reflect that `-u' is incompatibile\nwith explicit paths, or that `-u' does not require explicit paths\ncontrary to git-add without `-u'.\n\nThe fact that \"add --interactive does not take any parameters\" is\nseparate issue (which, accidentally, was adressed in the same patch).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"42080","messageId":"20070514003910.GA6689@coredump.intra.peff.net","threadId":"8100","inReplyTo":"200705131235.25281.jnareb@gmail.com","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-05-14T00:39:11Z","receivedAt":"2007-05-14T00:39:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 13, 2007 at 12:35:24PM +0200, Jakub Narebski wrote:\n\n> Do git-add *needs* path specifier (even if it is '.') also for `-u'?\n> The changes in documentation were to reflect that `-u' is incompatibile\n> with explicit paths, or that `-u' does not require explicit paths\n> contrary to git-add without `-u'.\n\nSorry, I don't understand if you are asking that question, or if you are\nasking if the documentation needs to clarify that point.\n\nThe answer to the first is \"no\", the path limiting works exactly as it\ndoes for every other command: no path specifier indicates the whole\ntree.\n\nIf you are concerned about the latter, do you mean something like this:\n\n-- >8 --\nDocumentation/git-add: clarify -u with path limiting\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex ea27018..27b9c0f 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -57,8 +57,11 @@ OPTIONS\n \tthe index.\n \n -u::\n-\tUpdate all files that git already knows about. This is what\n-\t\"git commit -a\" does in preparation for making a commit.\n+\tUpdate only files that git already knows about. This is similar\n+\tto what \"git commit -a\" does in preparation for making a commit,\n+\texcept that the update is limited to paths specified on the\n+\tcommand line. If no paths are specified, all tracked files are\n+\tupdated.\n \n \\--::\n \tThis option can be used to separate command-line options from\n"},{"id":"42082","messageId":"20070514004224.GB6689@coredump.intra.peff.net","threadId":"8100","inReplyTo":"200705131235.25281.jnareb@gmail.com","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-05-14T00:42:24Z","receivedAt":"2007-05-14T00:42:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, May 13, 2007 at 12:35:24PM +0200, Jakub Narebski wrote:\n\n> The fact that \"add --interactive does not take any parameters\" is\n> separate issue (which, accidentally, was adressed in the same patch).\n\nI don't generally use add --interactive, but I imagine that\npath-limiting would also make sense there. I think it would be a bit\nharder to implement (and test!), since there are many calls to commands\nwhich would need the limits. So I will leave that unless somebody really\ncares about it.\n\n-Peff\n"},{"id":"42084","messageId":"20070514004903.GC6689@coredump.intra.peff.net","threadId":"8100","inReplyTo":"200705140250.22478.jnareb@gmail.com","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-05-14T00:49:03Z","receivedAt":"2007-05-14T00:49:03Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 14, 2007 at 02:50:22AM +0200, Jakub Narebski wrote:\n\n> -'git-add' [-n] [-v] [-f] [--interactive | -i] [-u] [--] <file>...\n> +'git-add' [-n] [-v] [-f] [--] <file>...\n> +'git-add' [-n] [-v] [-f] -u [[--] <file>...]\n> +'git-add' (--interactive | -i)\n\nI don't see the point in splitting -u out as its own separate \"mode\" of\noperation. To me it's conceptually a flag that says \"don't add untracked\nfiles, and remove deleted files\". But I don't have a strong opinion (and\ncertainly splitting out -i makes sense).\n\n-Peff\n"},{"id":"42083","messageId":"200705140250.22478.jnareb@gmail.com","threadId":"8100","inReplyTo":"20070514003910.GA6689@coredump.intra.peff.net","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-05-14T00:50:22Z","receivedAt":"2007-05-14T00:50:22Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Jeff King wrote:\n\n> If you are concerned about the latter, do you mean something like this:\n> \n> -- >8 --\n> Documentation/git-add: clarify -u with path limiting\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> ---\n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index ea27018..27b9c0f 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -57,8 +57,11 @@ OPTIONS\n>  \tthe index.\n>  \n>  -u::\n> -\tUpdate all files that git already knows about. This is what\n> -\t\"git commit -a\" does in preparation for making a commit.\n> +\tUpdate only files that git already knows about. This is similar\n> +\tto what \"git commit -a\" does in preparation for making a commit,\n> +\texcept that the update is limited to paths specified on the\n> +\tcommand line. If no paths are specified, all tracked files are\n> +\tupdated.\n>  \n>  \\--::\n>  \tThis option can be used to separate command-line options from\n> \n\nThat is very nice and needed, but I actually thought about correcting\nSYNOPSIS to read:\n\n@@ -7,7 +7,9 @@ git-add - Add file contents to the changeset to be committed next\n \n SYNOPSIS\n --------\n-'git-add' [-n] [-v] [-f] [--interactive | -i] [-u] [--] <file>...\n+'git-add' [-n] [-v] [-f] [--] <file>...\n+'git-add' [-n] [-v] [-f] -u [[--] <file>...]\n+'git-add' (--interactive | -i)\n \n DESCRIPTION\n -----------\n\nThis ensures that while git-add without -u needs explicit paths\n(even if it is '.'), git-add with -u can have explicit paths but\ndoesn't need them.\n-- \nJakub Narebski\nPoland\n"},{"id":"42088","messageId":"7vmz08p558.fsf@assigned-by-dhcp.cox.net","threadId":"8100","inReplyTo":"20070514004903.GC6689@coredump.intra.peff.net","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-14T01:26:11Z","receivedAt":"2007-05-14T01:26:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Mon, May 14, 2007 at 02:50:22AM +0200, Jakub Narebski wrote:\n>\n>> -'git-add' [-n] [-v] [-f] [--interactive | -i] [-u] [--] <file>...\n>> +'git-add' [-n] [-v] [-f] [--] <file>...\n>> +'git-add' [-n] [-v] [-f] -u [[--] <file>...]\n>> +'git-add' (--interactive | -i)\n>\n> I don't see the point in splitting -u out as its own separate \"mode\" of\n> operation. To me it's conceptually a flag that says \"don't add untracked\n> files, and remove deleted files\". But I don't have a strong opinion (and\n> certainly splitting out -i makes sense).\n\nConcurred, and I agree with Jakub that your update to the\ndescription of -u makes it much better.\n\nThanks, both.\n"},{"id":"42089","messageId":"7virawp4zo.fsf@assigned-by-dhcp.cox.net","threadId":"8100","inReplyTo":"20070514004224.GB6689@coredump.intra.peff.net","subject":"Re: [PATCH] git-add: allow path limiting with -u","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-14T01:29:31Z","receivedAt":"2007-05-14T01:29:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Sun, May 13, 2007 at 12:35:24PM +0200, Jakub Narebski wrote:\n>\n>> The fact that \"add --interactive does not take any parameters\" is\n>> separate issue (which, accidentally, was adressed in the same patch).\n>\n> I don't generally use add --interactive, but I imagine that\n> path-limiting would also make sense there. I think it would be a bit\n> harder to implement (and test!), since there are many calls to commands\n> which would need the limits. So I will leave that unless somebody really\n> cares about it.\n\nI do not use -i myself that much, but due to its \"interactive\"\nnature, I would imagine that limiting path upfront is actually\ncounter-productive.  During the interactive refining process you\nmight notice that you wanted to handle some other paths you\nexcluded from the command line, but you won't be able to access\nthem if you limit upfront.  In other words, command line limiter\nwould only interfere with what the interactive user would really\nwant to do.\n"}]}