{"thread":{"id":"29135","subject":"[PATCH] Update documentation for stripspace","startedAt":"2011-12-12T01:59:18Z","lastAt":"2011-12-13T06:28:36Z","messageCount":6,"participants":["Conrad Irwin","Junio C Hamano","Frans Klaver"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"180840","messageId":"1323655158-5075-1-git-send-email-conrad.irwin@gmail.com","threadId":"29135","inReplyTo":null,"subject":"[PATCH] Update documentation for stripspace","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-12-12T01:59:18Z","receivedAt":"2011-12-12T01:59:18Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"Tell the user what this command is intended for, and expand the\ndescription of what it does.\n\nStop referring to the input as <stream>, as this command reads the\nentire input into memory before processing it.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-stripspace.txt |   26 ++++++++++++++++++++------\n builtin/stripspace.c             |    2 +-\n 2 files changed, 21 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt\nindex b78f031..6667d25 100644\n--- a/Documentation/git-stripspace.txt\n+++ b/Documentation/git-stripspace.txt\n@@ -3,26 +3,40 @@ git-stripspace(1)\n \n NAME\n ----\n-git-stripspace - Filter out empty lines\n+git-stripspace - Remove unnecessary whitespace\n \n \n SYNOPSIS\n --------\n [verse]\n-'git stripspace' [-s | --strip-comments] < <stream>\n+'git stripspace' [-s | --strip-comments] < input\n \n DESCRIPTION\n -----------\n-Remove multiple empty lines, and empty lines at beginning and end.\n+\n+Normalizes input in the manner used by 'git' for user-provided metadata such\n+as commit messages, notes, tags and branch descriptions.\n+\n+When run with no arguments this:\n+\n+- removes trailing whitespace from all lines\n+- collapses multiple consecutive empty lines into one empty line\n+- removes blank lines from the beginning and end of the input\n+- ensures the last line ends with exactly one '\\n'.\n+\n+In the case where the input consists entirely of whitespace characters, no\n+output will be produced.\n+\n+*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`\n+mode of linkgit:git-apply[1] for correcting whitespace of patches or files in\n+the repository.\n \n OPTIONS\n -------\n -s::\n --strip-comments::\n-\tIn addition to empty lines, also strip lines starting with '#'.\n+\tAlso remove all lines starting with '#'.\n \n-<stream>::\n-\tByte stream to act on.\n \n GIT\n ---\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 1288ffc..f16986c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -75,7 +75,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\t\t\t!strcmp(argv[1], \"--strip-comments\")))\n \t\tstrip_comments = 1;\n \telse if (argc > 1)\n-\t\tusage(\"git stripspace [-s | --strip-comments] < <stream>\");\n+\t\tusage(\"git stripspace [-s | --strip-comments] < input\");\n \n \tif (strbuf_read(&buf, 0, 1024) < 0)\n \t\tdie_errno(\"could not read the input\");\n-- \n1.7.8.164.g00d7e\n"},{"id":"180911","messageId":"7vy5ui5h0k.fsf@alter.siamese.dyndns.org","threadId":"29135","inReplyTo":"1323655158-5075-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH] Update documentation for stripspace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T06:41:31Z","receivedAt":"2011-12-12T06:41:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Conrad Irwin <conrad.irwin@gmail.com> writes:\n\n> Tell the user what this command is intended for, and expand the\n> description of what it does.\n\nThanks.\n\n> Stop referring to the input as <stream>, as this command reads the\n> entire input into memory before processing it.\n\nWhich can change to stream, but calling it as input would not invalidate\nthe new wording, so \"input\" is fine. From the caller's point of view, the\ncurrent implementation (or streaming implementation) can read from an\nunseekable input stream (i.e. pipe), so the original wording is equally\nvalid, by the way.\n\nSo in that sense, it does not make any difference either way to me (it is\nnot even worth rerolling this patch to only remove this part of the\nchange).\n\n> Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n> ---\n>  Documentation/git-stripspace.txt |   26 ++++++++++++++++++++------\n>  builtin/stripspace.c             |    2 +-\n>  2 files changed, 21 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt\n> index b78f031..6667d25 100644\n> --- a/Documentation/git-stripspace.txt\n> +++ b/Documentation/git-stripspace.txt\n> @@ -3,26 +3,40 @@ git-stripspace(1)\n>  \n>  NAME\n>  ----\n> -git-stripspace - Filter out empty lines\n> +git-stripspace - Remove unnecessary whitespace\n>  \n>  \n>  SYNOPSIS\n>  --------\n>  [verse]\n> -'git stripspace' [-s | --strip-comments] < <stream>\n> +'git stripspace' [-s | --strip-comments] < input\n>  \n>  DESCRIPTION\n>  -----------\n> -Remove multiple empty lines, and empty lines at beginning and end.\n> +\n> +Normalizes input in the manner used by 'git' for user-provided metadata such\n> +as commit messages, notes, tags and branch descriptions.\n\nThe original says \"remove\" and new one says \"normalize*s*\". I think we\ntend to say things in imperative mood (i.e. without the trailing \"s\").\n\nI do not think 'user-provided metadata' is a good wording. This is just a\nsimple text clean-up filter and you can use it to clean your text files\nthat you mean to store in the repository as well.\n\n> +When run with no arguments this:\n> +\n> +- removes trailing whitespace from all lines\n> +- collapses multiple consecutive empty lines into one empty line\n> +- removes blank lines from the beginning and end of the input\n> +- ensures the last line ends with exactly one '\\n'.\n\nThanks for a nicely written bulleted list. It clarifies what the command\ndoes quite a bit.\n\nThe last one is a bit funny, though.\n\nBy definition, you cannot end the last line with more than one '\\n' (upon\nseeing the second '\\n', you would realize immediately that the line you\nsaw was _not_ the last line). I think you meant the file does not end with\nan incomplete line, i.e. \"ensures the output does not end with an\nincomplete line by adding '\\n' at the end if needed\".\n\n> +In the case where the input consists entirely of whitespace characters, no\n> +output will be produced.\n> +\n> +*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`\n> +mode of linkgit:git-apply[1] for correcting whitespace of patches or files in\n> +the repository.\n\nI can tell that these three lines were the _primary_ thing you wanted to\nadd with this patch, having never seen anybody got confused between the\nwhitespace breakage fix and text cleaning, I wonder if this is adding\nclarity or giving users an impression that git can do too many things than\nthey can wrap their mind around and forcing them to wonder if they have to\nlearn everything git can do for them.\n\n>  OPTIONS\n>  -------\n>  -s::\n>  --strip-comments::\n> -\tIn addition to empty lines, also strip lines starting with '#'.\n> +\tAlso remove all lines starting with '#'.\n\nWith the resulting text (with the rules clarified with your above 4-bullet\npoints) of this manual page, can a user tell what the command does to this\ninput (I added line numbers, vertical bars and dollar signs to show where\nthe beginning and the end of lines are):\n\n    1 | $\n    2 |a b c$\n    3 |$\n    4 |# comment line$\n    5 |$\n    6 |d e f$\n    \nThe original text at least allows the user to guess correctly, as it hints\nthat a comment line is treated pretty much like an empty line, and the\n\"consecutive empty lines are squashed into one\" in your bulleted list\nwould mean that ll 3-5 will become a single blank line.\n\nThe new text however gives a wrong hint by saying \"Also\"; it can be read\nas if all the rules in the bullted list are applied first to leave blank\nlines at 3 and 5 and then comment line is removed from the result, which\nwould leave two blank lines in the result.\n\nIf I were touching this description, I probably would say something like\n\"Treat lines starting with a '#' as if they are empty lines\".\n"},{"id":"180982","messageId":"1323728909-7847-1-git-send-email-conrad.irwin@gmail.com","threadId":"29135","inReplyTo":"7vy5ui5h0k.fsf@alter.siamese.dyndns.org","subject":"[PATCH v2] Update documentation for stripspace","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-12-12T22:28:29Z","receivedAt":"2011-12-12T22:28:29Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"On Sun, Dec 11, 2011 at 10:41 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Conrad Irwin <conrad.irwin@gmail.com> writes:\n>\n> The original says \"remove\" and new one says \"normalize*s*\". I think we\n> tend to say things in imperative mood (i.e. without the trailing \"s\").\n\nGood point, fixed.\n\n>\n> I do not think 'user-provided metadata' is a good wording. This is just a\n> simple text clean-up filter and you can use it to clean your text files\n> that you mean to store in the repository as well.\n\nChanged just to \"text\", as that seems simpler. I've left the example\nuses as is, they were the prime reason for that sentence existing.\n\n>\n> The last one is a bit funny, though.\n>\n> By definition, you cannot end the last line with more than one '\\n' (upon\n> seeing the second '\\n', you would realize immediately that the line you\n> saw was _not_ the last line). I think you meant the file does not end with\n> an incomplete line, i.e. \"ensures the output does not end with an\n> incomplete line by adding '\\n' at the end if needed\".\n\nHmm, I'm not sure that's the best way of describing it — I've gone with:\n\"add a missing '\\n' to the last line if necessary.\".\n\n>\n>> +In the case where the input consists entirely of whitespace characters, no\n>> +output will be produced.\n>> +\n>> +*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`\n>> +mode of linkgit:git-apply[1] for correcting whitespace of patches or files in\n>> +the repository.\n>\n> I can tell that these three lines were the _primary_ thing you wanted to\n> add with this patch, having never seen anybody got confused between the\n> whitespace breakage fix and text cleaning, I wonder if this is adding\n> clarity or giving users an impression that git can do too many things than\n> they can wrap their mind around and forcing them to wonder if they have to\n> learn everything git can do for them.\n\nThe motivation for this patch was an old post to the Cairo mailing list\n[1]. There they were using (previously undocumented) behaviour of\ngit stripspace, instead of git apply --whitespace=fix (it may be that\n--whitespace=fix didn't exist at that point?).\n\nI've moved the *NOTE* into a SEE ALSO section where I think it reads\nless opinionatedly — is that better?\n\n>\n>>  OPTIONS\n>>  -------\n>>  -s::\n>>  --strip-comments::\n>> -     In addition to empty lines, also strip lines starting with '#'.\n>> +     Also remove all lines starting with '#'.\n[snip]\n>\n> If I were touching this description, I probably would say something like\n> \"Treat lines starting with a '#' as if they are empty lines\".\n>\n\nIf only it were that simple! If you have a commented line between two\nnon-commented lines, then no empty line results. I've added an example\nto the re-rolled patch that show-cases the behaviour of comment\nstripping in both cases. I went with \"skip and remove\" as the verb, to\nimply that the lines are ignored by the previous transformations.\n\nThanks for the detailed feedback.\n\nConrad\n\n[1] http://lists.freedesktop.org/archives/cairo/2006-June/007062.html\n\n----8<----\n\nTell the user what this command is intended for, and expand the\ndescription of what it does.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-stripspace.txt |   69 ++++++++++++++++++++++++++++++++++---\n builtin/stripspace.c             |    2 +-\n 2 files changed, 64 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt\nindex b78f031..a0a6ea4 100644\n--- a/Documentation/git-stripspace.txt\n+++ b/Documentation/git-stripspace.txt\n@@ -3,26 +3,83 @@ git-stripspace(1)\n \n NAME\n ----\n-git-stripspace - Filter out empty lines\n+git-stripspace - Remove unnecessary whitespace\n \n \n SYNOPSIS\n --------\n [verse]\n-'git stripspace' [-s | --strip-comments] < <stream>\n+'git stripspace' [-s | --strip-comments] < input\n \n DESCRIPTION\n -----------\n-Remove multiple empty lines, and empty lines at beginning and end.\n+\n+Clean the input in the manner used by 'git' for text such as commit\n+messages, notes, tags and branch descriptions.\n+\n+With no arguments, this will:\n+\n+- remove trailing whitespace from all lines\n+- collapse multiple consecutive empty lines into one empty line\n+- remove blank lines from the beginning and end of the input\n+- add a missing '\\n' to the last line if necessary.\n+\n+In the case where the input consists entirely of whitespace characters, no\n+output will be produced.\n \n OPTIONS\n -------\n -s::\n --strip-comments::\n-\tIn addition to empty lines, also strip lines starting with '#'.\n+\tSkip and remove all lines starting with '#'.\n+\n+EXAMPLES\n+--------\n+\n+Given the following noisy input with '$' indicating the end of a line:\n+\n+--------\n+|A brief introduction   $\n+|   $\n+|$\n+|A new paragraph$\n+|# with a commented-out line    $\n+|explaining lots of stuff.$\n+|$\n+|# An old paragraph, also commented-out. $\n+|      $\n+|The end.$\n+|  $\n+---------\n+\n+Use 'git stripspace' with no arguments to obtain:\n+\n+--------\n+|A brief introduction$\n+|$\n+|A new paragraph$\n+|# with a commented-out line$\n+|explaining lots of stuff.$\n+|$\n+|# An old paragraph, also commented-out.$\n+|$\n+|The end.$\n+---------\n \n-<stream>::\n-\tByte stream to act on.\n+Use 'git stripspace --strip-comments' to obtain:\n+\n+--------\n+|A brief introduction$\n+|$\n+|A new paragraph$\n+|explaining lots of stuff.$\n+|$\n+|The end.$\n+---------\n+\n+SEE ALSO\n+--------\n+The `--whitespace=fix` mode of linkgit:git-apply[1].\n \n GIT\n ---\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 1288ffc..f16986c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -75,7 +75,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\t\t\t!strcmp(argv[1], \"--strip-comments\")))\n \t\tstrip_comments = 1;\n \telse if (argc > 1)\n-\t\tusage(\"git stripspace [-s | --strip-comments] < <stream>\");\n+\t\tusage(\"git stripspace [-s | --strip-comments] < input\");\n \n \tif (strbuf_read(&buf, 0, 1024) < 0)\n \t\tdie_errno(\"could not read the input\");\n-- \n1.7.8.164.g00d7e\n"},{"id":"180992","messageId":"7vwra1z7bg.fsf@alter.siamese.dyndns.org","threadId":"29135","inReplyTo":"1323728909-7847-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v2] Update documentation for stripspace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-12-12T23:41:39Z","receivedAt":"2011-12-12T23:41:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Conrad Irwin <conrad.irwin@gmail.com> writes:\n\n> ...\n> I've moved the *NOTE* into a SEE ALSO section where I think it reads\n> less opinionatedly ― is that better?\n\nI think it looks a lot worse.\n\nAt least your original hinted that some people may confuse the two and the\nNOTE was there for such people; other people who would not even dream of\nsuch a confusion won't find the existence of the note a \"Huh?\". But the\nupdated patch with a link in SEE ALSO section without any explanation\nwould be a definite \"Huh?\" for those of us who find that stripspace does\nnot have anything to do with what \"apply --whitespace=fix\" does.\n\nThe new example section looks good. Perhaps we can just drop the extra SEE\nALSO and resurrect the *NOTE* from your v1 patch.\n\nThanks.\n"},{"id":"180996","messageId":"1323733971-12495-1-git-send-email-conrad.irwin@gmail.com","threadId":"29135","inReplyTo":"7vwra1z7bg.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Update documentation for stripspace","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-12-12T23:52:51Z","receivedAt":"2011-12-12T23:52:51Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"2011/12/12 Junio C Hamano <gitster@pobox.com>:\n> Conrad Irwin <conrad.irwin@gmail.com> writes:\n>> I've moved the *NOTE* into a SEE ALSO section where I think it reads\n>> less opinionatedly ― is that better?\n>\n> I think it looks a lot worse.\n[snip]\n>\n> The new example section looks good. Perhaps we can just drop the extra SEE\n> ALSO and resurrect the *NOTE* from your v1 patch.\n>\n> Thanks.\n\nDone.\n\nConrad\n\n---8<---\n\nTell the user what this command is intended for, and expand the\ndescription of what it does.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-stripspace.txt |   69 ++++++++++++++++++++++++++++++++++---\n builtin/stripspace.c             |    2 +-\n 2 files changed, 64 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt\nindex b78f031..a548444 100644\n--- a/Documentation/git-stripspace.txt\n+++ b/Documentation/git-stripspace.txt\n@@ -3,26 +3,83 @@ git-stripspace(1)\n \n NAME\n ----\n-git-stripspace - Filter out empty lines\n+git-stripspace - Remove unnecessary whitespace\n \n \n SYNOPSIS\n --------\n [verse]\n-'git stripspace' [-s | --strip-comments] < <stream>\n+'git stripspace' [-s | --strip-comments] < input\n \n DESCRIPTION\n -----------\n-Remove multiple empty lines, and empty lines at beginning and end.\n+\n+Clean the input in the manner used by 'git' for text such as commit\n+messages, notes, tags and branch descriptions.\n+\n+With no arguments, this will:\n+\n+- remove trailing whitespace from all lines\n+- collapse multiple consecutive empty lines into one empty line\n+- remove empty lines from the beginning and end of the input\n+- add a missing '\\n' to the last line if necessary.\n+\n+In the case where the input consists entirely of whitespace characters, no\n+output will be produced.\n+\n+*NOTE*: This is intended for cleaning metadata, prefer the `--whitespace=fix`\n+mode of linkgit:git-apply[1] for correcting whitespace of patches or files in\n+the repository.\n \n OPTIONS\n -------\n -s::\n --strip-comments::\n-\tIn addition to empty lines, also strip lines starting with '#'.\n+\tSkip and remove all lines starting with '#'.\n+\n+EXAMPLES\n+--------\n+\n+Given the following noisy input with '$' indicating the end of a line:\n \n-<stream>::\n-\tByte stream to act on.\n+--------\n+|A brief introduction   $\n+|   $\n+|$\n+|A new paragraph$\n+|# with a commented-out line    $\n+|explaining lots of stuff.$\n+|$\n+|# An old paragraph, also commented-out. $\n+|      $\n+|The end.$\n+|  $\n+---------\n+\n+Use 'git stripspace' with no arguments to obtain:\n+\n+--------\n+|A brief introduction$\n+|$\n+|A new paragraph$\n+|# with a commented-out line$\n+|explaining lots of stuff.$\n+|$\n+|# An old paragraph, also commented-out.$\n+|$\n+|The end.$\n+---------\n+\n+Use 'git stripspace --strip-comments' to obtain:\n+\n+--------\n+|A brief introduction$\n+|$\n+|A new paragraph$\n+|explaining lots of stuff.$\n+|$\n+|The end.$\n+---------\n \n GIT\n ---\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 1288ffc..f16986c 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -75,7 +75,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\t\t\t!strcmp(argv[1], \"--strip-comments\")))\n \t\tstrip_comments = 1;\n \telse if (argc > 1)\n-\t\tusage(\"git stripspace [-s | --strip-comments] < <stream>\");\n+\t\tusage(\"git stripspace [-s | --strip-comments] < input\");\n \n \tif (strbuf_read(&buf, 0, 1024) < 0)\n \t\tdie_errno(\"could not read the input\");\n-- \n1.7.8.164.g00d7e\n"},{"id":"181024","messageId":"op.v6ez9yyi0aolir@keputer","threadId":"29135","inReplyTo":"1323728909-7847-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v2] Update documentation for stripspace","fromName":"Frans Klaver","fromEmail":"fransklaver@gmail.com","sentAt":"2011-12-13T06:28:36Z","receivedAt":"2011-12-13T06:28:36Z","isPatch":true,"sender":{"key":"fransklaver@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1876483?v=4"},"body":"On Mon, 12 Dec 2011 23:28:29 +0100, Conrad Irwin <conrad.irwin@gmail.com>  \nwrote:\n\n>> an incomplete line, i.e. \"ensures the output does not end with an\n>> incomplete line by adding '\\n' at the end if needed\".\n>\n> Hmm, I'm not sure that's the best way of describing it — I've gone with:\n> \"add a missing '\\n' to the last line if necessary.\".\n\nIn most editors/IDE's I know and that support this, this is called \"ensure  \nnew-line at end of file\". I find this wording clearer than the above two  \noptions.\n\nCheers,\nFrans\n"}]}