{"thread":{"id":"33082","subject":"[PATCH] add: Clarify documentation of -A and -u","startedAt":"2013-03-06T07:26:10Z","lastAt":"2013-03-07T17:52:06Z","messageCount":4,"participants":["Greg Price","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"210674","messageId":"20130306072610.GB15534@biohazard-cafe.mit.edu","threadId":"33082","inReplyTo":null,"subject":"[PATCH] add: Clarify documentation of -A and -u","fromName":"Greg Price","fromEmail":"price@mit.edu","sentAt":"2013-03-06T07:26:10Z","receivedAt":"2013-03-06T07:26:10Z","isPatch":true,"sender":{"key":"price@mit.edu","avatar":"https://avatars.githubusercontent.com/u/28173?v=4"},"body":"The documentation of '-A' is very confusing for someone who doesn't\nalready know what it does. In particular, it describes the option's\nmeaning by reference to that of '-u', but it's no more similar to the\nlatter than it is to the default behavior. Describe it directly (and\nin a way that uses the word 'all'), and then describe the contrast\nseparately.\n\nSigned-off-by: Greg Price \n---\n Documentation/git-add.txt | 19 ++++++++++++++-----\n 1 file changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 388a225..f89d920 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -105,7 +105,9 @@ apply to the index. See EDITING PATCHES below.\n \twill never stage new files, but that it will stage modified\n \tnew contents of tracked files and that it will remove files\n \tfrom the index if the corresponding files in the working tree\n-\thave been removed.\n+\thave been removed. By contrast `-A` will add new files as\n+\twell as updating and removing existing ones, and the default\n+\tbehavior will add new files and will not remove existing ones.\n +\n If no is given, the current version of Git defaults to\n \".\"; in other words, update all tracked files in the current directory\n@@ -114,10 +116,17 @@ of Git, hence the form without should not be used.\n \n -A::\n --all::\n-\tLike `-u`, but match against files in the\n-\tworking tree in addition to the index. That means that it\n-\twill find new files as well as staging modified content and\n-\tremoving files that are no longer in the working tree.\n+\tUpdate the index regarding all files that match ,\n+\teither in the index or the working tree. That is, remove\n+\tfiles that are only in the index, add files that are only in\n+\tthe working tree, and update files that differ between the\n+\ttwo. By contrast `-u` only removes and updates, and the\n+\tdefault behavior only adds and updates.\n++\n+If no is given, the current version of Git defaults to\n+\".\"; in other words, update all tracked files in the current directory\n+and its subdirectories. This default will change in a future version\n+of Git, hence the form without should not be used.\n \n -N::\n --intent-to-add::\n-- \n1.7.11.3\n"},{"id":"210703","messageId":"7vhakoqwri.fsf@alter.siamese.dyndns.org","threadId":"33082","inReplyTo":"20130306072610.GB15534@biohazard-cafe.mit.edu","subject":"Re: [PATCH] add: Clarify documentation of -A and -u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-06T17:10:57Z","receivedAt":"2013-03-06T17:10:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Price writes:\n\n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index 388a225..f89d920 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -105,7 +105,9 @@ apply to the index. See EDITING PATCHES below.\n> \twill never stage new files, but that it will stage modified\n> \tnew contents of tracked files and that it will remove files\n> \tfrom the index if the corresponding files in the working tree\n> -\thave been removed.\n> +\thave been removed. By contrast `-A` will add new files as\n> +\twell as updating and removing existing ones, and the default\n> +\tbehavior will add new files and will not remove existing ones.\n> +\n> If no is given, the current version of Git defaults to\n> \".\"; in other words, update all tracked files in the current directory\n\nI do not know if mentioning what -A does in the description for -u\n(and vice versa) makes it easier to understand or more confusing\n(not rhetorical: I suspect some may find it easier and others not).\n\nBut \"and the default behaviour will...\" here _is_ confusing. After\nreading this patch three times, I still cannot tell what \"default\"\nyou are trying to describe. Is it \"-u\" without arguments? Is it\n\"add\" without \"-u\" nor \"-A\"? Is it something else???\n\nExactly the same comment applies to the other hunk.\n\nHaving said that, I agree that the current description for \"-u\" is\nway suboptimal. It begins like this:\n\n -u::\n --update::\n Only match against already tracked files in\n the index rather than the working tree. That means that it\n will ...\n\nWhenever you see an incomprehensible technobabble followed by \"That\nmeans\", \"In other words\", etc., you (not limited to Greg-you but\nfiguratively \"everybody\") should see if it makes it easier to read\nto remove everything up to that \"That means\" and adding a bit more\nto the remainder. In this particular case, the technobabble is not\neven correct---\"match\"ing is only a small part of what \"add -u\" does\nto find what to \"add\".\n\nI would suggest rewriting the first part of \"-u\" perhaps like this:\n\n\t-u::\n --update::\n\t\tUpdate modified and/or removed paths in the index\n\t\tthat match with the current state of the\n\t\tworking tree files. No new path is added because\n\t\tthis considers only the paths that are already in\n\t\tthe index.\n\n\nThe text for \"-A\" may look like this:\n\n\t-A::\n --all::\n\t\tUpdate the index to record the current state of the\n\t\tworking tree files that match . Note that\n\t\tnew paths will be added to the index, in addition to\n\t\tmodified and/or removed paths.\n\nI agree with your patch that \"If no is given\" should be\nrepeated for both \"-u\" and \"-A\".\n\n> @@ -114,10 +116,17 @@ of Git, hence the form without should not be used.\n> \n> -A::\n> --all::\n> -\tLike `-u`, but match against files in the\n> -\tworking tree in addition to the index. That means that it\n> -\twill find new files as well as staging modified content and\n> -\tremoving files that are no longer in the working tree.\n> +\tUpdate the index regarding all files that match ,\n> +\teither in the index or the working tree. That is, remove\n> +\tfiles that are only in the index, add files that are only in\n> +\tthe working tree, and update files that differ between the\n> +\ttwo. By contrast `-u` only removes and updates, and the\n> +\tdefault behavior only adds and updates.\n> ++\n> +If no is given, the current version of Git defaults to\n> +\".\"; in other words, update all tracked files in the current directory\n> +and its subdirectories. This default will change in a future version\n> +of Git, hence the form without should not be used.\n> \n> -N::\n> --intent-to-add::\n"},{"id":"210757","messageId":"20130307101314.GP22203@biohazard-cafe.mit.edu","threadId":"33082","inReplyTo":"7vhakoqwri.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] add: Clarify documentation of -A and -u","fromName":"Greg Price","fromEmail":"price@mit.edu","sentAt":"2013-03-07T10:13:15Z","receivedAt":"2013-03-07T10:13:15Z","isPatch":true,"sender":{"key":"price@mit.edu","avatar":"https://avatars.githubusercontent.com/u/28173?v=4"},"body":"On Wed, Mar 06, 2013 at 09:10:57AM -0800, Junio C Hamano wrote:\n> I do not know if mentioning what -A does in the description for -u\n> (and vice versa) makes it easier to understand or more confusing\n> (not rhetorical: I suspect some may find it easier and others not).\n> \n> But \"and the default behaviour will...\" here _is_ confusing. After\n> reading this patch three times, I still cannot tell what \"default\"\n> you are trying to describe. Is it \"-u\" without arguments? Is it\n> \"add\" without \"-u\" nor \"-A\"? Is it something else???\n\nI meant the behavior without -A or -u. This could be fixed directly\nby s/the default behavior will/with neither -A nor -u we/. But we can\nalso keep revising -- there's definitely more room to make this\nclearer! I suggest new versions at the bottom of this message.\n\nI do think that it's important that the reader be able to see clearly\nwhat the option does as a contrast with what the command does without\nthe option. Normally in a man page this is how the option is\ndescribed in the first place -- for example, the next entry in this\nvery man page says \"Record *only* the fact that the path will be added\nlater\", rather than \"Record the fact that ...\" These two descriptions\nsuffer from not doing that, so that it's not clear which parts of the\nbehavior they describe are just what 'add' does with no options and\nwhich parts are actually due to the option.\n\n\n> Whenever you see an incomprehensible technobabble followed by \"That\n> means\", \"In other words\", etc., you (not limited to Greg-you but\n> figuratively \"everybody\") should see if it makes it easier to read\n> to remove everything up to that \"That means\" [...]\n\nSeems like a reasonable heuristic.\n\n\n> \t-u::\n> --update::\n> \t\tUpdate modified and/or removed paths in the index\n> \t\tthat match with the current state of the\n> \t\tworking tree files. No new path is added because\n> \t\tthis considers only the paths that are already in\n> \t\tthe index.\n> \n> \t-A::\n> --all::\n> \t\tUpdate the index to record the current state of the\n> \t\tworking tree files that match . Note that\n> \t\tnew paths will be added to the index, in addition to\n> \t\tmodified and/or removed paths.\n\nThese are good -- I especially like that they're shorter. I do think\nthey're still likely to be confusing. The lead sentences are hard to\ntell apart from each other or one's mental model of what 'add' alone\ndoes, though the contrasts that follow them help. I also think the\nlead sentence for '--all' isn't really correct -- we update the index\nnot only for the working tree files that match , but also\nwhere there is no working tree file, only an index entry. (So the\nsentence actually describes what 'add' with neither option does.)\n\nMaybe it's worth taking a step back. The overall taxonomy is\n * 'add' alone considers matching filenames in the working tree\n * 'add -u' considers matching filenames in the index\n * 'add -A' considers matching filenames in both the index and the\n working tree\nand in each case we make the index match the working tree on those\nfiles. Or, put another way,\n * 'add' alone modifies and adds files\n * 'add -u' modifies and removes files\n * 'add -A' modifies, adds, and removes files\n\nHere's a crack at making those distinctions clear. I've also tried to\nmake the descriptions as parallel as possible, as what they're saying\nis very similar.\n\n-u::\n--update::\n\tUpdate the index just where it already has an entry matching\n\t. This removes as well as modifies index entries to\n\tmatch the working tree, but adds no new files.\n\n-A::\n--all::\n\tUpdate the index not only where the working tree has a file\n\tmatching but also where the index already has an\n\tentry. This adds, modifies, and removes index entries to\n\tmatch the working tree.\n\nThese are the shortest in the discussion so far, and I think they're\nalso the clearest.\n\nThen follow both with the \"If no \" paragraph. I just\nnoticed that the paragraph actually needs a small modification to fit\n'-A', too. New patch below.\n\nGreg\n\n\nFrom: Greg Price \nDate: Thu, 7 Mar 2013 02:08:21 -0800\nSubject: [PATCH] add: Clarify documentation of -A and -u\n\nThe documentation of '-A' and '-u' is very confusing for someone who\ndoesn't already know what they do. Describe them with fewer words and\nclearer parallelism to each other and to the behavior of plain 'add'.\n\nAlso mention the default for '-A' as well as '-u', because\nit applies to both.\n\nSigned-off-by: Greg Price \n---\n Documentation/git-add.txt | 22 ++++++++++++----------\n 1 file changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 388a225..b0944e5 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -100,12 +100,9 @@ apply to the index. See EDITING PATCHES below.\n \n -u::\n --update::\n-\tOnly match against already tracked files in\n-\tthe index rather than the working tree. That means that it\n-\twill never stage new files, but that it will stage modified\n-\tnew contents of tracked files and that it will remove files\n-\tfrom the index if the corresponding files in the working tree\n-\thave been removed.\n+\tUpdate the index just where it already has an entry matching\n+\t. This removes as well as modifies index entries to\n+\tmatch the working tree, but adds no new files.\n +\n If no is given, the current version of Git defaults to\n \".\"; in other words, update all tracked files in the current directory\n@@ -114,10 +111,15 @@ of Git, hence the form without should not be used.\n \n -A::\n --all::\n-\tLike `-u`, but match against files in the\n-\tworking tree in addition to the index. That means that it\n-\twill find new files as well as staging modified content and\n-\tremoving files that are no longer in the working tree.\n+\tUpdate the index not only where the working tree has a file\n+\tmatching but also where the index already has an\n+\tentry.\tThis adds, modifies, and removes index entries to\n+\tmatch the working tree.\n++\n+If no is given, the current version of Git defaults to\n+\".\"; in other words, update all files in the current directory\n+and its subdirectories. This default will change in a future version\n+of Git, hence the form without should not be used.\n \n -N::\n --intent-to-add::\n-- \n1.7.11.3\n"},{"id":"210774","messageId":"7vsj47m721.fsf@alter.siamese.dyndns.org","threadId":"33082","inReplyTo":"20130307101314.GP22203@biohazard-cafe.mit.edu","subject":"Re: [PATCH] add: Clarify documentation of -A and -u","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-07T17:52:06Z","receivedAt":"2013-03-07T17:52:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Price writes:\n\n> On Wed, Mar 06, 2013 at 09:10:57AM -0800, Junio C Hamano wrote:\n>> I do not know if mentioning what -A does in the description for -u\n>> (and vice versa) makes it easier to understand or more confusing\n>> (not rhetorical: I suspect some may find it easier and others not).\n>> \n>> But \"and the default behaviour will...\" here _is_ confusing. After\n>> reading this patch three times, I still cannot tell what \"default\"\n>> you are trying to describe. Is it \"-u\" without arguments? Is it\n>> \"add\" without \"-u\" nor \"-A\"? Is it something else???\n>\n> I meant the behavior without -A or -u. This could be fixed directly\n> by s/the default behavior will/with neither -A nor -u we/.\n\nWhen we have bulletted list that enumerates options and describes\nwhat each option does and how each option affects the behaviour, I'd\nprefer to see us *not* talking about what happens when you do *not*\ngive that option, unless it makes it hard to understand that option\nwithout such an extra description. The overall description of what\nthe command does without the options should go to the top level.\n\n> Here's a crack at making those distinctions clear. I've also tried to\n> make the descriptions as parallel as possible, as what they're saying\n> is very similar.\n>\n> -u::\n> --update::\n> \tUpdate the index just where it already has an entry matching\n> \t.\n\nGood; this was the short phrasing I was looking for but couldn't\ncome up with myself without repeating \"the index\" over and over.\n\n> Then follow both with the \"If no \" paragraph. I just\n> noticed that the paragraph actually needs a small modification to fit\n> '-A', too. New patch below.\n>\n> Greg\n>\n> From: Greg Price \n> Date: Thu, 7 Mar 2013 02:08:21 -0800\n> Subject: [PATCH] add: Clarify documentation of -A and -u\n\n(for future reference) Drop the three lines and have \"-- >8 --\" here.\n\n[patch kept unsnipped for others']\n\n> The documentation of '-A' and '-u' is very confusing for someone who\n> doesn't already know what they do. Describe them with fewer words and\n> clearer parallelism to each other and to the behavior of plain 'add'.\n>\n> Also mention the default for '-A' as well as '-u', because\n> it applies to both.\n>\n> Signed-off-by: Greg Price \n> ---\n> Documentation/git-add.txt | 22 ++++++++++++----------\n> 1 file changed, 12 insertions(+), 10 deletions(-)\n>\n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index 388a225..b0944e5 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -100,12 +100,9 @@ apply to the index. See EDITING PATCHES below.\n> \n> -u::\n> --update::\n> -\tOnly match against already tracked files in\n> -\tthe index rather than the working tree. That means that it\n> -\twill never stage new files, but that it will stage modified\n> -\tnew contents of tracked files and that it will remove files\n> -\tfrom the index if the corresponding files in the working tree\n> -\thave been removed.\n> +\tUpdate the index just where it already has an entry matching\n> +\t. This removes as well as modifies index entries to\n> +\tmatch the working tree, but adds no new files.\n> +\n> If no is given, the current version of Git defaults to\n> \".\"; in other words, update all tracked files in the current directory\n> @@ -114,10 +111,15 @@ of Git, hence the form without should not be used.\n> \n> -A::\n> --all::\n> -\tLike `-u`, but match against files in the\n> -\tworking tree in addition to the index. That means that it\n> -\twill find new files as well as staging modified content and\n> -\tremoving files that are no longer in the working tree.\n> +\tUpdate the index not only where the working tree has a file\n> +\tmatching but also where the index already has an\n> +\tentry.\tThis adds, modifies, and removes index entries to\n> +\tmatch the working tree.\n> ++\n> +If no is given, the current version of Git defaults to\n> +\".\"; in other words, update all files in the current directory\n> +and its subdirectories. This default will change in a future version\n> +of Git, hence the form without should not be used.\n> \n> -N::\n> --intent-to-add::\n"}]}