{"thread":{"id":"40573","subject":"[PATCH v2 0/4] stripspace: Implement and use --count-lines option","startedAt":"2015-10-16T15:16:41Z","lastAt":"2015-10-20T15:47:50Z","messageCount":23,"participants":["Tobias Klauser","Junio C Hamano","Matthieu Moy","Eric Sunshine","Christian Couder"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"271809","messageId":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","threadId":"40573","inReplyTo":null,"subject":"[PATCH v2 0/4] stripspace: Implement and use --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-16T15:16:41Z","receivedAt":"2015-10-16T15:16:41Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"(1) Move the stripspace() function to the strbuf module adding a prefix\n and changing all users accordingly. Also introduce a wrapper in case\n any topic branches still depend on the old name.\n\n(2) Switch git stripspace to use parse-options in order to simplify\n introducing new command line options (as in the following patch). In\n v1 this was folded into patch (3) and is now split out for v2.\n\n(3) Introduce option --count-lines to git stripspace and add the\n corresponding documentation and tests.\n\n(4) Change git-rebase--interactive.sh to replace commands like:\n\n\tgit stripspace ... | wc -l\n\n with:\n\n\tgit stripspace --count-lines ...\n\nThis patch set implements some of the project ideas around git stripspace\nsuggested on https://git.wiki.kernel.org/index.php/SmallProjectsIdeas\n\nv1 -> v2:\n\n - Thanks to Junio and Matthieu for the review.\n - Split patch 2/3 into two patches: patch 2/4 switches git stripspace\n to use parse-options and patch 3/4 introduces the new option.\n - Implement line counting in cmd_stripbuf() instead of (ab-)using\n strbuf_stripspace() for it.\n - Drop -C short option\n - Correct example command output in documentation.\n - Adjust commit messages to not include links to the wiki, fully\n describe the motivation in the commit message instead.\n\nTobias Klauser (4):\n strbuf: make stripspace() part of strbuf\n stripspace: Use parse-options for command-line parsing\n stripspace: Implement --count-lines option\n git rebase -i: Use newly added --count-lines option for stripspace\n\n Documentation/git-stripspace.txt | 14 +++-\n builtin/am.c | 2 +-\n builtin/branch.c | 2 +-\n builtin/commit.c | 6 +-\n builtin/merge.c | 2 +-\n builtin/notes.c | 6 +-\n builtin/stripspace.c | 137 +++++++++++++--------------------------\n builtin/tag.c | 2 +-\n git-rebase--interactive.sh | 6 +-\n strbuf.c | 66 +++++++++++++++++++\n strbuf.h | 11 +++-\n t/t0030-stripspace.sh | 36 ++++++++++\n 12 files changed, 181 insertions(+), 109 deletions(-)\n\n-- \n2.6.1.148.g7927db1\n"},{"id":"271810","messageId":"1445008605-16534-2-git-send-email-tklauser@distanz.ch","threadId":"40573","inReplyTo":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","subject":"[PATCH v2 1/4] strbuf: make stripspace() part of strbuf","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-16T15:16:42Z","receivedAt":"2015-10-16T15:16:42Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"Rename stripspace() to strbuf_stripspace() and move it to the strbuf\nmodule. The function is also used in other builtins than stripspace, so\nit makes sense to have it in a more generic place. Since it operates on\nan strbuf and the function is declared in strbuf.h, move it to strbuf.c\nand add the corresponding prefix to its name.\n\nAlso switch all current users of stripspace() to the new function name\nand keep a temporary wrapper inline function for any topic branches\nstill using stripspace().\n\nReviewed-by: Matthieu Moy \nSigned-off-by: Tobias Klauser \n---\n\nImplements the small project idea from\nhttps://git.wiki.kernel.org/index.php/SmallProjectsIdeas#make_.27stripspace.28.29.27_part_of_strbuf\n\n builtin/am.c | 2 +-\n builtin/branch.c | 2 +-\n builtin/commit.c | 6 ++---\n builtin/merge.c | 2 +-\n builtin/notes.c | 6 ++---\n builtin/stripspace.c | 69 ++--------------------------------------------------\n builtin/tag.c | 2 +-\n strbuf.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++\n strbuf.h | 11 ++++++++-\n 9 files changed, 88 insertions(+), 78 deletions(-)\n\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 3bd4fd7..7b8e11e 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -1343,7 +1343,7 @@ static int parse_mail(struct am_state *state, const char *mail)\n \tstrbuf_addstr(&msg, \"\\n\\n\");\n \tif (strbuf_read_file(&msg, am_path(state, \"msg\"), 0) < 0)\n \t\tdie_errno(_(\"could not read '%s'\"), am_path(state, \"msg\"));\n-\tstripspace(&msg, 0);\n+\tstrbuf_stripspace(&msg, 0);\n \n \tif (state->signoff)\n \t\tam_signoff(&msg);\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 01f9530..b99a436 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -592,7 +592,7 @@ static int edit_branch_description(const char *branch_name)\n \t\tstrbuf_release(&buf);\n \t\treturn -1;\n \t}\n-\tstripspace(&buf, 1);\n+\tstrbuf_stripspace(&buf, 1);\n \n \tstrbuf_addf(&name, \"branch.%s.description\", branch_name);\n \tstatus = git_config_set(name.buf, buf.len ? buf.buf : NULL);\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 63772d0..dca09e2 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -775,7 +775,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \ts->hints = 0;\n \n \tif (clean_message_contents)\n-\t\tstripspace(&sb, 0);\n+\t\tstrbuf_stripspace(&sb, 0);\n \n \tif (signoff)\n \t\tappend_signoff(&sb, ignore_non_trailer(&sb), 0);\n@@ -1014,7 +1014,7 @@ static int template_untouched(struct strbuf *sb)\n \tif (!template_file || strbuf_read_file(&tmpl, template_file, 0) <= 0)\n \t\treturn 0;\n \n-\tstripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n+\tstrbuf_stripspace(&tmpl, cleanup_mode == CLEANUP_ALL);\n \tif (!skip_prefix(sb->buf, tmpl.buf, &start))\n \t\tstart = sb->buf;\n \tstrbuf_release(&tmpl);\n@@ -1726,7 +1726,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\twt_status_truncate_message_at_cut_line(&sb);\n \n \tif (cleanup_mode != CLEANUP_NONE)\n-\t\tstripspace(&sb, cleanup_mode == CLEANUP_ALL);\n+\t\tstrbuf_stripspace(&sb, cleanup_mode == CLEANUP_ALL);\n \tif (template_untouched(&sb) && !allow_empty_message) {\n \t\trollback_index_files();\n \t\tfprintf(stderr, _(\"Aborting commit; you did not edit the message.\\n\"));\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex a0edaca..e6741f3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -806,7 +806,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t\t\tabort_commit(remoteheads, NULL);\n \t}\n \tread_merge_msg(&msg);\n-\tstripspace(&msg, 0 < option_edit);\n+\tstrbuf_stripspace(&msg, 0 < option_edit);\n \tif (!msg.len)\n \t\tabort_commit(remoteheads, _(\"Empty commit message.\"));\n \tstrbuf_release(&merge_msg);\ndiff --git a/builtin/notes.c b/builtin/notes.c\nindex 3608c64..bb23d55 100644\n--- a/builtin/notes.c\n+++ b/builtin/notes.c\n@@ -192,7 +192,7 @@ static void prepare_note_data(const unsigned char *object, struct note_data *d,\n \t\tif (launch_editor(d->edit_path, &d->buf, NULL)) {\n \t\t\tdie(_(\"Please supply the note contents using either -m or -F option\"));\n \t\t}\n-\t\tstripspace(&d->buf, 1);\n+\t\tstrbuf_stripspace(&d->buf, 1);\n \t}\n }\n \n@@ -215,7 +215,7 @@ static int parse_msg_arg(const struct option *opt, const char *arg, int unset)\n \tif (d->buf.len)\n \t\tstrbuf_addch(&d->buf, '\\n');\n \tstrbuf_addstr(&d->buf, arg);\n-\tstripspace(&d->buf, 0);\n+\tstrbuf_stripspace(&d->buf, 0);\n \n \td->given = 1;\n \treturn 0;\n@@ -232,7 +232,7 @@ static int parse_file_arg(const struct option *opt, const char *arg, int unset)\n \t\t\tdie_errno(_(\"cannot read '%s'\"), arg);\n \t} else if (strbuf_read_file(&d->buf, arg, 1024) < 0)\n \t\tdie_errno(_(\"could not open or read '%s'\"), arg);\n-\tstripspace(&d->buf, 0);\n+\tstrbuf_stripspace(&d->buf, 0);\n \n \td->given = 1;\n \treturn 0;\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex 1259ed7..f677093 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -1,71 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n-\n-/*\n- * Returns the length of a line, without trailing spaces.\n- *\n- * If the line ends with newline, it will be removed too.\n- */\n-static size_t cleanup(char *line, size_t len)\n-{\n-\twhile (len) {\n-\t\tunsigned char c = line[len - 1];\n-\t\tif (!isspace(c))\n-\t\t\tbreak;\n-\t\tlen--;\n-\t}\n-\n-\treturn len;\n-}\n-\n-/*\n- * Remove empty lines from the beginning and end\n- * and also trailing spaces from every line.\n- *\n- * Turn multiple consecutive empty lines between paragraphs\n- * into just one empty line.\n- *\n- * If the input has only empty lines and spaces,\n- * no output will be produced.\n- *\n- * If last line does not have a newline at the end, one is added.\n- *\n- * Enable skip_comments to skip every line starting with comment\n- * character.\n- */\n-void stripspace(struct strbuf *sb, int skip_comments)\n-{\n-\tint empties = 0;\n-\tsize_t i, j, len, newlen;\n-\tchar *eol;\n-\n-\t/* We may have to add a newline. */\n-\tstrbuf_grow(sb, 1);\n-\n-\tfor (i = j = 0; i < sb->len; i += len, j += newlen) {\n-\t\teol = memchr(sb->buf + i, '\\n', sb->len - i);\n-\t\tlen = eol ? eol - (sb->buf + i) + 1 : sb->len - i;\n-\n-\t\tif (skip_comments && len && sb->buf[i] == comment_line_char) {\n-\t\t\tnewlen = 0;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tnewlen = cleanup(sb->buf + i, len);\n-\n-\t\t/* Not just an empty line? */\n-\t\tif (newlen) {\n-\t\t\tif (empties > 0 && j > 0)\n-\t\t\t\tsb->buf[j++] = '\\n';\n-\t\t\tempties = 0;\n-\t\t\tmemmove(sb->buf + j, sb->buf + i, newlen);\n-\t\t\tsb->buf[newlen + j++] = '\\n';\n-\t\t} else {\n-\t\t\tempties++;\n-\t\t}\n-\t}\n-\n-\tstrbuf_setlen(sb, j);\n-}\n+#include \"strbuf.h\"\n \n static void comment_lines(struct strbuf *buf)\n {\n@@ -111,7 +46,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\tdie_errno(\"could not read the input\");\n \n \tif (mode == STRIP_SPACE)\n-\t\tstripspace(&buf, strip_comments);\n+\t\tstrbuf_stripspace(&buf, strip_comments);\n \telse\n \t\tcomment_lines(&buf);\n \ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 9e17dca..5660787 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -268,7 +268,7 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n \n \tif (opt->cleanup_mode != CLEANUP_NONE)\n-\t\tstripspace(buf, opt->cleanup_mode == CLEANUP_ALL);\n+\t\tstrbuf_stripspace(buf, opt->cleanup_mode == CLEANUP_ALL);\n \n \tif (!opt->message_given && !buf->len)\n \t\tdie(_(\"no tag message?\"));\ndiff --git a/strbuf.c b/strbuf.c\nindex 29df55b..9583875 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -743,3 +743,69 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm)\n \t}\n \tstrbuf_setlen(sb, sb->len + len);\n }\n+\n+/*\n+ * Returns the length of a line, without trailing spaces.\n+ *\n+ * If the line ends with newline, it will be removed too.\n+ */\n+static size_t cleanup(char *line, size_t len)\n+{\n+\twhile (len) {\n+\t\tunsigned char c = line[len - 1];\n+\t\tif (!isspace(c))\n+\t\t\tbreak;\n+\t\tlen--;\n+\t}\n+\n+\treturn len;\n+}\n+\n+/*\n+ * Remove empty lines from the beginning and end\n+ * and also trailing spaces from every line.\n+ *\n+ * Turn multiple consecutive empty lines between paragraphs\n+ * into just one empty line.\n+ *\n+ * If the input has only empty lines and spaces,\n+ * no output will be produced.\n+ *\n+ * If last line does not have a newline at the end, one is added.\n+ *\n+ * Enable skip_comments to skip every line starting with comment\n+ * character.\n+ */\n+void strbuf_stripspace(struct strbuf *sb, int skip_comments)\n+{\n+\tint empties = 0;\n+\tsize_t i, j, len, newlen;\n+\tchar *eol;\n+\n+\t/* We may have to add a newline. */\n+\tstrbuf_grow(sb, 1);\n+\n+\tfor (i = j = 0; i < sb->len; i += len, j += newlen) {\n+\t\teol = memchr(sb->buf + i, '\\n', sb->len - i);\n+\t\tlen = eol ? eol - (sb->buf + i) + 1 : sb->len - i;\n+\n+\t\tif (skip_comments && len && sb->buf[i] == comment_line_char) {\n+\t\t\tnewlen = 0;\n+\t\t\tcontinue;\n+\t\t}\n+\t\tnewlen = cleanup(sb->buf + i, len);\n+\n+\t\t/* Not just an empty line? */\n+\t\tif (newlen) {\n+\t\t\tif (empties > 0 && j > 0)\n+\t\t\t\tsb->buf[j++] = '\\n';\n+\t\t\tempties = 0;\n+\t\t\tmemmove(sb->buf + j, sb->buf + i, newlen);\n+\t\t\tsb->buf[newlen + j++] = '\\n';\n+\t\t} else {\n+\t\t\tempties++;\n+\t\t}\n+\t}\n+\n+\tstrbuf_setlen(sb, j);\n+}\ndiff --git a/strbuf.h b/strbuf.h\nindex aef2794..5397d91 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -418,7 +418,16 @@ extern void strbuf_add_absolute_path(struct strbuf *sb, const char *path);\n * Strip whitespace from a buffer. The second parameter controls if\n * comments are considered contents to be removed or not.\n */\n-extern void stripspace(struct strbuf *buf, int skip_comments);\n+extern void strbuf_stripspace(struct strbuf *buf, int skip_comments);\n+\n+/**\n+ * Temporary alias until all topic branches have switched to use\n+ * strbuf_stripspace directly.\n+ */\n+static inline void stripspace(struct strbuf *buf, int skip_comments)\n+{\n+\tstrbuf_stripspace(buf, skip_comments);\n+}\n \n static inline int strbuf_strip_suffix(struct strbuf *sb, const char *suffix)\n {\n-- \n2.6.1.148.g7927db1\n"},{"id":"271813","messageId":"1445008605-16534-3-git-send-email-tklauser@distanz.ch","threadId":"40573","inReplyTo":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","subject":"[PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-16T15:16:43Z","receivedAt":"2015-10-16T15:16:43Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"Use parse-options to parse command-line options instead of a\nhand-crafted implementation.\n\nThis is a preparatory patch to simplify the introduction of the\n--count-lines option in a follow-up patch.\n\nSigned-off-by: Tobias Klauser \n---\n builtin/stripspace.c | 56 ++++++++++++++++++++++++++++------------------------\n 1 file changed, 30 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex f677093..ac1ab3d 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -1,5 +1,6 @@\n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"strbuf.h\"\n \n static void comment_lines(struct strbuf *buf)\n@@ -12,41 +13,44 @@ static void comment_lines(struct strbuf *buf)\n \tfree(msg);\n }\n \n-static const char *usage_msg = \"\\n\"\n-\" git stripspace [-s | --strip-comments] < input\\n\"\n-\" git stripspace [-c | --comment-lines] < input\";\n+static const char * const stripspace_usage[] = {\n+\tN_(\"git stripspace [-s | --strip-comments] < input\"),\n+\tN_(\"git stripspace [-c | --comment-lines] < input\"),\n+\tNULL\n+};\n+\n+enum stripspace_mode {\n+\tSTRIP_DEFAULT = 0,\n+\tSTRIP_COMMENTS,\n+\tCOMMENT_LINES\n+};\n \n int cmd_stripspace(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n-\tint strip_comments = 0;\n-\tenum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = STRIP_SPACE;\n-\n-\tif (argc == 2) {\n-\t\tif (!strcmp(argv[1], \"-s\") ||\n-\t\t !strcmp(argv[1], \"--strip-comments\")) {\n-\t\t\tstrip_comments = 1;\n-\t\t} else if (!strcmp(argv[1], \"-c\") ||\n-\t\t\t !strcmp(argv[1], \"--comment-lines\")) {\n-\t\t\tmode = COMMENT_LINES;\n-\t\t} else {\n-\t\t\tmode = INVAL;\n-\t\t}\n-\t} else if (argc > 1) {\n-\t\tmode = INVAL;\n-\t}\n-\n-\tif (mode == INVAL)\n-\t\tusage(usage_msg);\n-\n-\tif (strip_comments || mode == COMMENT_LINES)\n+\tenum stripspace_mode mode = STRIP_DEFAULT;\n+\n+\tconst struct option options[] = {\n+\t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n+\t\t\t N_(\"skip and remove all lines starting with comment character\"),\n+\t\t\t STRIP_COMMENTS),\n+\t\tOPT_CMDMODE('c', \"comment-lines\", &mode,\n+\t\t\t N_(\"prepend comment character and blank to each line\"),\n+\t\t\t COMMENT_LINES),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options, stripspace_usage,\n+\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\n+\tif (mode == STRIP_COMMENTS || mode == COMMENT_LINES)\n \t\tgit_config(git_default_config, NULL);\n \n \tif (strbuf_read(&buf, 0, 1024) < 0)\n \t\tdie_errno(\"could not read the input\");\n \n-\tif (mode == STRIP_SPACE)\n-\t\tstrbuf_stripspace(&buf, strip_comments);\n+\tif (mode == STRIP_DEFAULT || mode == STRIP_COMMENTS)\n+\t\tstrbuf_stripspace(&buf, mode == STRIP_COMMENTS);\n \telse\n \t\tcomment_lines(&buf);\n \n-- \n2.6.1.148.g7927db1\n"},{"id":"271811","messageId":"1445008605-16534-4-git-send-email-tklauser@distanz.ch","threadId":"40573","inReplyTo":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","subject":"[PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-16T15:16:44Z","receivedAt":"2015-10-16T15:16:44Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"Implement the --count-lines options for git stripspace to be able to\nomit calling:\n\n git stripspace --strip-comments < infile | wc -l\n\ne.g. in git-rebase--interactive.sh. The above command can now be\nreplaced by:\n\n git stripspace --strip-comments --count-lines < infile\n\nThis will make it easier to port git-rebase--interactive.sh to C later\non.\n\nFurthermore, add the corresponding documentation and tests.\n\nSigned-off-by: Tobias Klauser \n---\n\nImplements the small project idea from\nhttps://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27\n\n Documentation/git-stripspace.txt | 14 ++++++++++++--\n builtin/stripspace.c | 18 +++++++++++++++---\n t/t0030-stripspace.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 63 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-stripspace.txt b/Documentation/git-stripspace.txt\nindex 60328d5..79900b8 100644\n--- a/Documentation/git-stripspace.txt\n+++ b/Documentation/git-stripspace.txt\n@@ -9,8 +9,8 @@ git-stripspace - Remove unnecessary whitespace\n SYNOPSIS\n --------\n [verse]\n-'git stripspace' [-s | --strip-comments] < input\n-'git stripspace' [-c | --comment-lines] < input\n+'git stripspace' [-s | --strip-comments] [--count-lines] < input\n+'git stripspace' [-c | --comment-lines] [--count-lines] < input\n \n DESCRIPTION\n -----------\n@@ -44,6 +44,10 @@ OPTIONS\n \tbe terminated with a newline. On empty lines, only the comment character\n \twill be prepended.\n \n+--count-lines::\n+\tOutput the number of resulting lines after stripping. This is equivalent\n+\tto calling 'git stripspace | wc -l'.\n+\n EXAMPLES\n --------\n \n@@ -88,6 +92,12 @@ Use 'git stripspace --strip-comments' to obtain:\n |The end.$\n ---------\n \n+Use 'git stripspace --count-lines' to obtain:\n+\n+---------\n+5\n+---------\n+\n GIT\n ---\n Part of the linkgit:git[1] suite\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex ac1ab3d..487523f 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -14,8 +14,8 @@ static void comment_lines(struct strbuf *buf)\n }\n \n static const char * const stripspace_usage[] = {\n-\tN_(\"git stripspace [-s | --strip-comments] < input\"),\n-\tN_(\"git stripspace [-c | --comment-lines] < input\"),\n+\tN_(\"git stripspace [-s | --strip-comments] [--count-lines] < input\"),\n+\tN_(\"git stripspace [-c | --comment-lines] [--count-lines] < input\"),\n \tNULL\n };\n \n@@ -29,6 +29,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tenum stripspace_mode mode = STRIP_DEFAULT;\n+\tint count_lines = 0;\n \n \tconst struct option options[] = {\n \t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n@@ -37,6 +38,7 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\tOPT_CMDMODE('c', \"comment-lines\", &mode,\n \t\t\t N_(\"prepend comment character and blank to each line\"),\n \t\t\t COMMENT_LINES),\n+\t\tOPT_BOOL(0, \"count-lines\", &count_lines, N_(\"print line count\")),\n \t\tOPT_END()\n \t};\n \n@@ -54,7 +56,17 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \telse\n \t\tcomment_lines(&buf);\n \n-\twrite_or_die(1, buf.buf, buf.len);\n+\tif (!count_lines)\n+\t\twrite_or_die(1, buf.buf, buf.len);\n+\telse {\n+\t\tsize_t i, lines;\n+\n+\t\tfor (i = lines = 0; i < buf.len; i++) {\n+\t\t\tif (buf.buf[i] == '\\n')\n+\t\t\t\tlines++;\n+\t\t}\n+\t\tprintf(\"%zu\\n\", lines);\n+\t}\n \tstrbuf_release(&buf);\n \treturn 0;\n }\ndiff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\nindex 29e91d8..9c00cb9 100755\n--- a/t/t0030-stripspace.sh\n+++ b/t/t0030-stripspace.sh\n@@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented line' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success '--count-lines with newline only' '\n+\tprintf \"0\\n\" >expect &&\n+\tprintf \"\\n\" | git stripspace --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--count-lines with single line' '\n+\tprintf \"1\\n\" >expect &&\n+\tprintf \"foo\\n\" | git stripspace --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--count-lines with single line preceeded by empty line' '\n+\tprintf \"1\\n\" >expect &&\n+\tprintf \"\\nfoo\" | git stripspace --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--count-lines with single line followed by empty line' '\n+\tprintf \"1\\n\" >expect &&\n+\tprintf \"foo\\n\\n\" | git stripspace --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--count-lines with multiple lines and consecutive newlines' '\n+\tprintf \"5\\n\" >expect &&\n+\tprintf \"\\none\\n\\n\\nthree\\nfour\\nfive\\n\" | git stripspace --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--count-lines combined with --strip-comments' '\n+\tprintf \"5\\n\" >expect &&\n+\tprintf \"\\n# stripped\\none\\n#stripped\\n\\nthree\\nfour\\nfive\\n\" | git stripspace -s --count-lines >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.6.1.148.g7927db1\n"},{"id":"271812","messageId":"1445008605-16534-5-git-send-email-tklauser@distanz.ch","threadId":"40573","inReplyTo":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","subject":"[PATCH v2 4/4] git rebase -i: Use newly added --count-lines option for stripspace","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-16T15:16:45Z","receivedAt":"2015-10-16T15:16:45Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"Use the newly added --count-lines option for 'git stripspace' to count\nlines instead of piping the entire output to 'wc -l'.\n\nSigned-off-by: Tobias Klauser \n---\n\nImplements the small project idea from\nhttps://git.wiki.kernel.org/index.php/SmallProjectsIdeas#implement_.27--count-lines.27_in_.27git_stripspace.27\n\n git-rebase--interactive.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex d65c06e..f80da30 100644\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -120,9 +120,9 @@ mark_action_done () {\n \tsed -e 1q < \"$todo\" >> \"$done\"\n \tsed -e 1d < \"$todo\" >> \"$todo\".new\n \tmv -f \"$todo\".new \"$todo\"\n-\tnew_count=$(git stripspace --strip-comments <\"$done\" | wc -l)\n+\tnew_count=$(git stripspace --strip-comments --count-lines <\"$done\")\n \techo $new_count >\"$msgnum\"\n-\ttotal=$(($new_count + $(git stripspace --strip-comments <\"$todo\" | wc -l)))\n+\ttotal=$(($new_count + $(git stripspace --strip-comments --count-lines <\"$todo\")))\n \techo $total >\"$end\"\n \tif test \"$last_count\" != \"$new_count\"\n \tthen\n@@ -1243,7 +1243,7 @@ test -s \"$todo\" || echo noop >> \"$todo\"\n test -n \"$autosquash\" && rearrange_squash \"$todo\"\n test -n \"$cmd\" && add_exec_commands \"$todo\"\n \n-todocount=$(git stripspace --strip-comments <\"$todo\" | wc -l)\n+todocount=$(git stripspace --strip-comments --count-lines <\"$todo\")\n todocount=${todocount##* }\n \n cat >>\"$todo\" < writes:\n\nBe consistent with the subjects, please.\n\n> strbuf: make stripspace() part of strbuf\n\ns/make/make/ ;-)\n\n> stripspace: Use parse-options for command-line parsing\n\ns/Use/use/\n\n> stripspace: Implement --count-lines option\n\ns/Implement/implement/\n\n> git rebase -i: Use newly added --count-lines option for stripspace\n\ns/Use/use/\n\n\n> Documentation/git-stripspace.txt | 14 +++-\n> builtin/am.c | 2 +-\n> builtin/branch.c | 2 +-\n> builtin/commit.c | 6 +-\n> builtin/merge.c | 2 +-\n> builtin/notes.c | 6 +-\n> builtin/stripspace.c | 137 +++++++++++++--------------------------\n> builtin/tag.c | 2 +-\n> git-rebase--interactive.sh | 6 +-\n> strbuf.c | 66 +++++++++++++++++++\n> strbuf.h | 11 +++-\n> t/t0030-stripspace.sh | 36 ++++++++++\n> 12 files changed, 181 insertions(+), 109 deletions(-)\n"},{"id":"271819","messageId":"vpqsi5a69ey.fsf@grenoble-inp.fr","threadId":"40573","inReplyTo":"1445008605-16534-1-git-send-email-tklauser@distanz.ch","subject":"Re: [PATCH v2 0/4] stripspace: Implement and use --count-lines option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-16T16:54:45Z","receivedAt":"2015-10-16T16:54:45Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Tobias Klauser writes:\n\n> - Split patch 2/3 into two patches: patch 2/4 switches git stripspace\n> to use parse-options and patch 3/4 introduces the new option.\n\nMuch better now.\n\n> - Implement line counting in cmd_stripbuf() instead of (ab-)using\n> strbuf_stripspace() for it.\n\nAlso short and sweet, I like it.\n\n> - Drop -C short option\n> - Correct example command output in documentation.\n> - Adjust commit messages to not include links to the wiki, fully\n> describe the motivation in the commit message instead.\n\nGood.\n\nI read the patches again, and the whole series is now\n\nReviewed-by: Matthieu Moy \n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271821","messageId":"xmqqoafyg2sp.fsf@gitster.mtv.corp.google.com","threadId":"40573","inReplyTo":"1445008605-16534-3-git-send-email-tklauser@distanz.ch","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T17:07:34Z","receivedAt":"2015-10-16T17:07:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tobias Klauser writes:\n\n> Use parse-options to parse command-line options instead of a\n> hand-crafted implementation.\n>\n> This is a preparatory patch to simplify the introduction of the\n> --count-lines option in a follow-up patch.\n\nThe second paragraph is probably of much lessor importance than one\nthing you forgot to mention: the users can now use a unique prefix\nof the option and say \"stripspace --comment\".\n\n> +enum stripspace_mode {\n> +\tSTRIP_DEFAULT = 0,\n> +\tSTRIP_COMMENTS,\n> +\tCOMMENT_LINES\n> +};\n> \n> int cmd_stripspace(int argc, const char **argv, const char *prefix)\n> {\n> \tstruct strbuf buf = STRBUF_INIT;\n> -\tint strip_comments = 0;\n> -\tenum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = STRIP_SPACE;\n> -\n> -\tif (argc == 2) {\n> -\t\tif (!strcmp(argv[1], \"-s\") ||\n> -\t\t !strcmp(argv[1], \"--strip-comments\")) {\n> -\t\t\tstrip_comments = 1;\n> -\t\t} else if (!strcmp(argv[1], \"-c\") ||\n> -\t\t\t !strcmp(argv[1], \"--comment-lines\")) {\n> -\t\t\tmode = COMMENT_LINES;\n> -\t\t} else {\n> -\t\t\tmode = INVAL;\n> -\t\t}\n> -\t} else if (argc > 1) {\n> -\t\tmode = INVAL;\n> -\t}\n> -\n> -\tif (mode == INVAL)\n> -\t\tusage(usage_msg);\n\nWhen given \"git stripspace -s blorg\", we used to set mode to INVAL\nand then showed the correct usage. But we no longer have a check\nthat corresponds to the old INVAL thing, do we? Perhaps check argc\nto detect presence of an otherwise ignored non-option argument\nimmediately after parse_options() returns?\n\n> -\tif (strip_comments || mode == COMMENT_LINES)\n> +\tenum stripspace_mode mode = STRIP_DEFAULT;\n> +\n> +\tconst struct option options[] = {\n> +\t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n> +\t\t\t N_(\"skip and remove all lines starting with comment character\"),\n> +\t\t\t STRIP_COMMENTS),\n> +\t\tOPT_CMDMODE('c', \"comment-lines\", &mode,\n> +\t\t\t N_(\"prepend comment character and blank to each line\"),\n> +\t\t\t COMMENT_LINES),\n> +\t\tOPT_END()\n> +\t};\n> +\n> +\targc = parse_options(argc, argv, prefix, options, stripspace_usage,\n> +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n\nWhat is the point of keep-dashdash here?\n"},{"id":"271824","messageId":"xmqqd1weg1s0.fsf@gitster.mtv.corp.google.com","threadId":"40573","inReplyTo":"xmqqoafyg2sp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-16T17:29:35Z","receivedAt":"2015-10-16T17:29:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n>> -\tif (mode == INVAL)\n>> -\t\tusage(usage_msg);\n>\n> When given \"git stripspace -s blorg\", we used to set mode to INVAL\n> and then showed the correct usage. But we no longer have a check\n> that corresponds to the old INVAL thing, do we? Perhaps check argc\n> to detect presence of an otherwise ignored non-option argument\n> immediately after parse_options() returns?\n\nPerhaps like this.\n\ndiff --git a/builtin/stripspace.c b/builtin/stripspace.c\nindex ac1ab3d..a8b7a93 100644\n--- a/builtin/stripspace.c\n+++ b/builtin/stripspace.c\n@@ -40,8 +40,9 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n \t\tOPT_END()\n \t};\n \n-\targc = parse_options(argc, argv, prefix, options, stripspace_usage,\n-\t\t\t PARSE_OPT_KEEP_DASHDASH);\n+\targc = parse_options(argc, argv, prefix, options, stripspace_usage, 0);\n+\tif (argc)\n+\t\tusage_with_options(stripspace_usage, options);\n \n \tif (mode == STRIP_COMMENTS || mode == COMMENT_LINES)\n \t\tgit_config(git_default_config, NULL);\n"},{"id":"271849","messageId":"20151017102742.GA2468@distanz.ch","threadId":"40573","inReplyTo":"xmqqsi5ag404.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 0/4] stripspace: Implement and use --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-17T10:27:43Z","receivedAt":"2015-10-17T10:27:43Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-16 at 18:41:31 +0200, Junio C Hamano wrote:\n> Tobias Klauser writes:\n> \n> Be consistent with the subjects, please.\n> \n> > strbuf: make stripspace() part of strbuf\n> \n> s/make/make/ ;-)\n> \n> > stripspace: Use parse-options for command-line parsing\n> \n> s/Use/use/\n> \n> > stripspace: Implement --count-lines option\n> \n> s/Implement/implement/\n> \n> > git rebase -i: Use newly added --count-lines option for stripspace\n> \n> s/Use/use/\n\nWill adjust all of them to lowercase for v3. Thanks.\n"},{"id":"271848","messageId":"20151017102809.GB2468@distanz.ch","threadId":"40573","inReplyTo":"vpqsi5a69ey.fsf@grenoble-inp.fr","subject":"Re: [PATCH v2 0/4] stripspace: Implement and use --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-17T10:28:09Z","receivedAt":"2015-10-17T10:28:09Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-16 at 18:54:45 +0200, Matthieu Moy wrote:\n> Tobias Klauser writes:\n> \n> > - Split patch 2/3 into two patches: patch 2/4 switches git stripspace\n> > to use parse-options and patch 3/4 introduces the new option.\n> \n> Much better now.\n> \n> > - Implement line counting in cmd_stripbuf() instead of (ab-)using\n> > strbuf_stripspace() for it.\n> \n> Also short and sweet, I like it.\n> \n> > - Drop -C short option\n> > - Correct example command output in documentation.\n> > - Adjust commit messages to not include links to the wiki, fully\n> > describe the motivation in the commit message instead.\n> \n> Good.\n> \n> I read the patches again, and the whole series is now\n> \n> Reviewed-by: Matthieu Moy \n\nThank you for the review!\n"},{"id":"271850","messageId":"20151017103032.GC2468@distanz.ch","threadId":"40573","inReplyTo":"xmqqoafyg2sp.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-17T10:30:32Z","receivedAt":"2015-10-17T10:30:32Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-16 at 19:07:34 +0200, Junio C Hamano wrote:\n> Tobias Klauser writes:\n> \n> > Use parse-options to parse command-line options instead of a\n> > hand-crafted implementation.\n> >\n> > This is a preparatory patch to simplify the introduction of the\n> > --count-lines option in a follow-up patch.\n> \n> The second paragraph is probably of much lessor importance than one\n> thing you forgot to mention: the users can now use a unique prefix\n> of the option and say \"stripspace --comment\".\n\nI didn't even know about that feature, but now that you've mentioned it\nI will certainly make use of it more in the future :) And of course,\nI'll adjust the commit message accordingly for v3.\n\n> > +enum stripspace_mode {\n> > +\tSTRIP_DEFAULT = 0,\n> > +\tSTRIP_COMMENTS,\n> > +\tCOMMENT_LINES\n> > +};\n> > \n> > int cmd_stripspace(int argc, const char **argv, const char *prefix)\n> > {\n> > \tstruct strbuf buf = STRBUF_INIT;\n> > -\tint strip_comments = 0;\n> > -\tenum { INVAL = 0, STRIP_SPACE = 1, COMMENT_LINES = 2 } mode = STRIP_SPACE;\n> > -\n> > -\tif (argc == 2) {\n> > -\t\tif (!strcmp(argv[1], \"-s\") ||\n> > -\t\t !strcmp(argv[1], \"--strip-comments\")) {\n> > -\t\t\tstrip_comments = 1;\n> > -\t\t} else if (!strcmp(argv[1], \"-c\") ||\n> > -\t\t\t !strcmp(argv[1], \"--comment-lines\")) {\n> > -\t\t\tmode = COMMENT_LINES;\n> > -\t\t} else {\n> > -\t\t\tmode = INVAL;\n> > -\t\t}\n> > -\t} else if (argc > 1) {\n> > -\t\tmode = INVAL;\n> > -\t}\n> > -\n> > -\tif (mode == INVAL)\n> > -\t\tusage(usage_msg);\n> \n> When given \"git stripspace -s blorg\", we used to set mode to INVAL\n> and then showed the correct usage. But we no longer have a check\n> that corresponds to the old INVAL thing, do we? Perhaps check argc\n> to detect presence of an otherwise ignored non-option argument\n> immediately after parse_options() returns?\n\nAgreed, we should check it. I'll go with the implementation you\nsuggested in the follow-up message.\n\n> > -\tif (strip_comments || mode == COMMENT_LINES)\n> > +\tenum stripspace_mode mode = STRIP_DEFAULT;\n> > +\n> > +\tconst struct option options[] = {\n> > +\t\tOPT_CMDMODE('s', \"strip-comments\", &mode,\n> > +\t\t\t N_(\"skip and remove all lines starting with comment character\"),\n> > +\t\t\t STRIP_COMMENTS),\n> > +\t\tOPT_CMDMODE('c', \"comment-lines\", &mode,\n> > +\t\t\t N_(\"prepend comment character and blank to each line\"),\n> > +\t\t\t COMMENT_LINES),\n> > +\t\tOPT_END()\n> > +\t};\n> > +\n> > +\targc = parse_options(argc, argv, prefix, options, stripspace_usage,\n> > +\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> \n> What is the point of keep-dashdash here?\n\nLikewise, it shouldn't be there as in your follow-up patch.\n"},{"id":"271851","messageId":"20151017103134.GD2468@distanz.ch","threadId":"40573","inReplyTo":"xmqqd1weg1s0.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-17T10:31:35Z","receivedAt":"2015-10-17T10:31:35Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-16 at 19:29:35 +0200, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n> >> -\tif (mode == INVAL)\n> >> -\t\tusage(usage_msg);\n> >\n> > When given \"git stripspace -s blorg\", we used to set mode to INVAL\n> > and then showed the correct usage. But we no longer have a check\n> > that corresponds to the old INVAL thing, do we? Perhaps check argc\n> > to detect presence of an otherwise ignored non-option argument\n> > immediately after parse_options() returns?\n> \n> Perhaps like this.\n\nThanks. I'll fold it into v3.\n\n> diff --git a/builtin/stripspace.c b/builtin/stripspace.c\n> index ac1ab3d..a8b7a93 100644\n> --- a/builtin/stripspace.c\n> +++ b/builtin/stripspace.c\n> @@ -40,8 +40,9 @@ int cmd_stripspace(int argc, const char **argv, const char *prefix)\n> \t\tOPT_END()\n> \t};\n> \n> -\targc = parse_options(argc, argv, prefix, options, stripspace_usage,\n> -\t\t\t PARSE_OPT_KEEP_DASHDASH);\n> +\targc = parse_options(argc, argv, prefix, options, stripspace_usage, 0);\n> +\tif (argc)\n> +\t\tusage_with_options(stripspace_usage, options);\n> \n> \tif (mode == STRIP_COMMENTS || mode == COMMENT_LINES)\n> \t\tgit_config(git_default_config, NULL);\n> \n"},{"id":"271860","messageId":"xmqq6125choi.fsf@gitster.mtv.corp.google.com","threadId":"40573","inReplyTo":"20151017103134.GD2468@distanz.ch","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-17T21:24:13Z","receivedAt":"2015-10-17T21:24:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tobias Klauser writes:\n\n> On 2015-10-16 at 19:29:35 +0200, Junio C Hamano wrote:\n>> Junio C Hamano writes:\n>> \n>> >> -\tif (mode == INVAL)\n>> >> -\t\tusage(usage_msg);\n>> >\n>> > When given \"git stripspace -s blorg\", we used to set mode to INVAL\n>> > and then showed the correct usage. But we no longer have a check\n>> > that corresponds to the old INVAL thing, do we? Perhaps check argc\n>> > to detect presence of an otherwise ignored non-option argument\n>> > immediately after parse_options() returns?\n>> \n>> Perhaps like this.\n>\n> Thanks. I'll fold it into v3.\n\nBefore starting v3, please fetch from me and check what is queued on\n'pu'. It may turn out that the fix-ups I did while queuing this\nround is sufficient, in which case you can just say that instead ;-)\n\nThanks.\n"},{"id":"271862","messageId":"CAPig+cQ=8FO8yFY4sHUwr0mYuyvMu4d-eizHZeadE9f0BgpXpQ@mail.gmail.com","threadId":"40573","inReplyTo":"1445008605-16534-4-git-send-email-tklauser@distanz.ch","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-17T23:57:57Z","receivedAt":"2015-10-17T23:57:57Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Oct 16, 2015 at 11:16 AM, Tobias Klauser wrote:\n> Implement the --count-lines options for git stripspace [...]\n>\n> This will make it easier to port git-rebase--interactive.sh to C later\n> on.\n\nIs there any application beyond git-rebase--interactive where a\n--count-lines options is expected to be useful? It's not obvious from\nthe commit message that this change is necessarily a win for later\nporting of git-rebase--interactive to C since the amount of extra code\nand support material added by this patch probably outweighs the amount\nof code a C version of git-rebase--interactive would need to count the\nlines itself.\n\nStated differently, are the two or three instances of piping through\n'wc' in git-rebase--interactive sufficient justification for\nintroducing extra complexity into git-stripspace and its documentation\nand tests?\n\nMore below.\n\n> Furthermore, add the corresponding documentation and tests.\n>\n> Signed-off-by: Tobias Klauser \n> ---\n> diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\n> index 29e91d8..9c00cb9 100755\n> --- a/t/t0030-stripspace.sh\n> +++ b/t/t0030-stripspace.sh\n> @@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented line' '\n> test_cmp expect actual\n> '\n>\n> +test_expect_success '--count-lines with newline only' '\n> + printf \"0\\n\" >expect &&\n> + printf \"\\n\" | git stripspace --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n\nWhat is the expected behavior when the input is an empty file, a file\nwith content but no newline, a file with one or more lines but lacking\na newline on the final line? Should these cases be tested, as well?\n\n> +test_expect_success '--count-lines with single line' '\n> + printf \"1\\n\" >expect &&\n> + printf \"foo\\n\" | git stripspace --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success '--count-lines with single line preceeded by empty line' '\n> + printf \"1\\n\" >expect &&\n> + printf \"\\nfoo\" | git stripspace --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success '--count-lines with single line followed by empty line' '\n> + printf \"1\\n\" >expect &&\n> + printf \"foo\\n\\n\" | git stripspace --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success '--count-lines with multiple lines and consecutive newlines' '\n> + printf \"5\\n\" >expect &&\n> + printf \"\\none\\n\\n\\nthree\\nfour\\nfive\\n\" | git stripspace --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success '--count-lines combined with --strip-comments' '\n> + printf \"5\\n\" >expect &&\n> + printf \"\\n# stripped\\none\\n#stripped\\n\\nthree\\nfour\\nfive\\n\" | git stripspace -s --count-lines >actual &&\n> + test_cmp expect actual\n> +'\n> +\n> test_done\n> --\n> 2.6.1.148.g7927db1\n"},{"id":"271878","messageId":"xmqqwpukayde.fsf@gitster.mtv.corp.google.com","threadId":"40573","inReplyTo":"CAPig+cQ=8FO8yFY4sHUwr0mYuyvMu4d-eizHZeadE9f0BgpXpQ@mail.gmail.com","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-18T17:18:53Z","receivedAt":"2015-10-18T17:18:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine writes:\n\n> Is there any application beyond git-rebase--interactive where a\n> --count-lines options is expected to be useful? It's not obvious from\n> the commit message that this change is necessarily a win for later\n> porting of git-rebase--interactive to C since the amount of extra code\n> and support material added by this patch probably outweighs the amount\n> of code a C version of git-rebase--interactive would need to count the\n> lines itself.\n>\n> Stated differently, are the two or three instances of piping through\n> 'wc' in git-rebase--interactive sufficient justification for\n> introducing extra complexity into git-stripspace and its documentation\n> and tests?\n\nInteresting thought. When somebody rewrites \"rebase -i\" in C,\nnobody needs to count lines in \"stripspace\" output. The rewritten\n\"rebase -i\" would internally run strbuf_stripspace() and the question\nbecomes what is the best way to let that code find out how many lines\nthe result contains.\n\nWhen viewed from that angle, I agree that \"stripspace --count\" does\nnot add anything to further the goal of helping \"rebase -i\" to move\nto C. Adding strbuf_count_lines() that counts the number of lines\nin the given strbuf (if there is no such helper yet; I didn't check),\nthough.\n\n>> +test_expect_success '--count-lines with newline only' '\n>> + printf \"0\\n\" >expect &&\n>> + printf \"\\n\" | git stripspace --count-lines >actual &&\n>> + test_cmp expect actual\n>> +'\n>\n> What is the expected behavior when the input is an empty file, a file\n> with content but no newline, a file with one or more lines but lacking\n> a newline on the final line? Should these cases be tested, as well?\n\nGood point here, too. If we were to add strbuf_count_lines()\nhelper, whoever adds that function needs to take a possible\nincomplete line at the end into account.\n\nThanks for your comments.\n"},{"id":"271933","messageId":"20151019133150.GK2468@distanz.ch","threadId":"40573","inReplyTo":"CAPig+cQ=8FO8yFY4sHUwr0mYuyvMu4d-eizHZeadE9f0BgpXpQ@mail.gmail.com","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-19T13:31:51Z","receivedAt":"2015-10-19T13:31:51Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-18 at 01:57:57 +0200, Eric Sunshine wrote:\n> On Fri, Oct 16, 2015 at 11:16 AM, Tobias Klauser wrote:\n> > Implement the --count-lines options for git stripspace [...]\n> >\n> > This will make it easier to port git-rebase--interactive.sh to C later\n> > on.\n> \n> Is there any application beyond git-rebase--interactive where a\n> --count-lines options is expected to be useful? It's not obvious from\n> the commit message that this change is necessarily a win for later\n> porting of git-rebase--interactive to C since the amount of extra code\n> and support material added by this patch probably outweighs the amount\n> of code a C version of git-rebase--interactive would need to count the\n> lines itself.\n\nAgreed, it doesn't make much sense anymore in the current form. An\nstrbuf helper function implementing the line counting would probably be\nthe better way. But I guess this should only be introduced once someone\ndecides to write a C version of git-rebase--interactive (or any other\nuse for line counting appears).\n\n> Stated differently, are the two or three instances of piping through\n> 'wc' in git-rebase--interactive sufficient justification for\n> introducing extra complexity into git-stripspace and its documentation\n> and tests?\n\nIMO it doesn't add a lot of complexity, but on the other hand it also\ndoesn't provide a large benefit apart from getting rid of a few\ncalls to an external program in a code path which is not performance\ncritical.\n\nSo I suggest I'll drop patches 3/4 and 4/4 for v3. Once someone really\nneeds the line counting functionality in C, an strbuf helper can still\nbe added.\n\n> More below.\n> \n> > Furthermore, add the corresponding documentation and tests.\n> >\n> > Signed-off-by: Tobias Klauser \n> > ---\n> > diff --git a/t/t0030-stripspace.sh b/t/t0030-stripspace.sh\n> > index 29e91d8..9c00cb9 100755\n> > --- a/t/t0030-stripspace.sh\n> > +++ b/t/t0030-stripspace.sh\n> > @@ -438,4 +438,40 @@ test_expect_success 'avoid SP-HT sequence in commented line' '\n> > test_cmp expect actual\n> > '\n> >\n> > +test_expect_success '--count-lines with newline only' '\n> > + printf \"0\\n\" >expect &&\n> > + printf \"\\n\" | git stripspace --count-lines >actual &&\n> > + test_cmp expect actual\n> > +'\n> \n> What is the expected behavior when the input is an empty file, a file\n> with content but no newline, a file with one or more lines but lacking\n> a newline on the final line? Should these cases be tested, as well?\n\nNot really sure. For the implementation I followed the behavior of 'wc\n-l' which doesn't consider the final line if it lacks a newline. Should\nthis be different for git's purposes? In any case, I agree that these\ncases should excplicitely be tested/documented.\n"},{"id":"271934","messageId":"20151019134633.GL2468@distanz.ch","threadId":"40573","inReplyTo":"xmqqwpukayde.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-19T13:46:34Z","receivedAt":"2015-10-19T13:46:34Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-18 at 19:18:53 +0200, Junio C Hamano wrote:\n> Eric Sunshine writes:\n> \n> > Is there any application beyond git-rebase--interactive where a\n> > --count-lines options is expected to be useful? It's not obvious from\n> > the commit message that this change is necessarily a win for later\n> > porting of git-rebase--interactive to C since the amount of extra code\n> > and support material added by this patch probably outweighs the amount\n> > of code a C version of git-rebase--interactive would need to count the\n> > lines itself.\n> >\n> > Stated differently, are the two or three instances of piping through\n> > 'wc' in git-rebase--interactive sufficient justification for\n> > introducing extra complexity into git-stripspace and its documentation\n> > and tests?\n> \n> Interesting thought. When somebody rewrites \"rebase -i\" in C,\n> nobody needs to count lines in \"stripspace\" output. The rewritten\n> \"rebase -i\" would internally run strbuf_stripspace() and the question\n> becomes what is the best way to let that code find out how many lines\n> the result contains.\n> \n> When viewed from that angle, I agree that \"stripspace --count\" does\n> not add anything to further the goal of helping \"rebase -i\" to move\n> to C. Adding strbuf_count_lines() that counts the number of lines\n> in the given strbuf (if there is no such helper yet; I didn't check),\n> though.\n\nI check before implementing this series and didn't find any helper. I\nalso didn't find any other uses of line counting in the code.\n\nAfter considering your and Eric's reply, I'll drop these patches for\nnow and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to\nEric).\n\n> >> +test_expect_success '--count-lines with newline only' '\n> >> + printf \"0\\n\" >expect &&\n> >> + printf \"\\n\" | git stripspace --count-lines >actual &&\n> >> + test_cmp expect actual\n> >> +'\n> >\n> > What is the expected behavior when the input is an empty file, a file\n> > with content but no newline, a file with one or more lines but lacking\n> > a newline on the final line? Should these cases be tested, as well?\n> \n> Good point here, too. If we were to add strbuf_count_lines()\n> helper, whoever adds that function needs to take a possible\n> incomplete line at the end into account.\n\nYes, makes more sense like this (even though it doesn't correspond to\nwhat 'wc -l' does).\n"},{"id":"271936","messageId":"CAP8UFD2pqg_J36V9wZkAR0b-L421gHFi9SbRqFwBbZ1LMVOKSg@mail.gmail.com","threadId":"40573","inReplyTo":"20151019134633.GL2468@distanz.ch","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2015-10-19T17:03:34Z","receivedAt":"2015-10-19T17:03:34Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser wrote:\n> On 2015-10-18 at 19:18:53 +0200, Junio C Hamano wrote:\n>> Eric Sunshine writes:\n>>\n>> > Is there any application beyond git-rebase--interactive where a\n>> > --count-lines options is expected to be useful? It's not obvious from\n>> > the commit message that this change is necessarily a win for later\n>> > porting of git-rebase--interactive to C since the amount of extra code\n>> > and support material added by this patch probably outweighs the amount\n>> > of code a C version of git-rebase--interactive would need to count the\n>> > lines itself.\n>> >\n>> > Stated differently, are the two or three instances of piping through\n>> > 'wc' in git-rebase--interactive sufficient justification for\n>> > introducing extra complexity into git-stripspace and its documentation\n>> > and tests?\n>>\n>> Interesting thought. When somebody rewrites \"rebase -i\" in C,\n>> nobody needs to count lines in \"stripspace\" output. The rewritten\n>> \"rebase -i\" would internally run strbuf_stripspace() and the question\n>> becomes what is the best way to let that code find out how many lines\n>> the result contains.\n>>\n>> When viewed from that angle, I agree that \"stripspace --count\" does\n>> not add anything to further the goal of helping \"rebase -i\" to move\n>> to C. Adding strbuf_count_lines() that counts the number of lines\n>> in the given strbuf (if there is no such helper yet; I didn't check),\n>> though.\n>\n> I check before implementing this series and didn't find any helper. I\n> also didn't find any other uses of line counting in the code.\n\nThis shows that implementing \"git stripspace --count-lines\" could\nindirectly help porting \"git rebase -i\" to C as you could implement\nstrbuf_count_lines() for the former and it could then be reused in the\nlatter.\n\n> After considering your and Eric's reply, I'll drop these patches for\n> now and only resubmit patches 1/4 and 2/4 for v3 (also see my reply to\n> Eric).\n\nIt would be sad in my opinion.\n\n>> >> +test_expect_success '--count-lines with newline only' '\n>> >> + printf \"0\\n\" >expect &&\n>> >> + printf \"\\n\" | git stripspace --count-lines >actual &&\n>> >> + test_cmp expect actual\n>> >> +'\n>> >\n>> > What is the expected behavior when the input is an empty file, a file\n>> > with content but no newline, a file with one or more lines but lacking\n>> > a newline on the final line? Should these cases be tested, as well?\n>>\n>> Good point here, too. If we were to add strbuf_count_lines()\n>> helper, whoever adds that function needs to take a possible\n>> incomplete line at the end into account.\n>\n> Yes, makes more sense like this (even though it doesn't correspond to\n> what 'wc -l' does).\n\nTests for \"git stripspace --count-lines\" would test\nstrbuf_count_lines() which would also help when porting git rebase -i\nto C.\n"},{"id":"271945","messageId":"CAPig+cR4wyumSfzXjptCfniuN0QC8TErL1X9LDPMsCD8wHP_kA@mail.gmail.com","threadId":"40573","inReplyTo":"CAP8UFD2pqg_J36V9wZkAR0b-L421gHFi9SbRqFwBbZ1LMVOKSg@mail.gmail.com","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-19T19:24:05Z","receivedAt":"2015-10-19T19:24:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Oct 19, 2015 at 1:03 PM, Christian Couder\n wrote:\n> On Mon, Oct 19, 2015 at 3:46 PM, Tobias Klauser wrote:\n>> On 2015-10-18 at 19:18:53 +0200, Junio C Hamano wrote:\n>>> Eric Sunshine writes:\n>>> > Is there any application beyond git-rebase--interactive where a\n>>> > --count-lines options is expected to be useful? It's not obvious from\n>>> > the commit message that this change is necessarily a win for later\n>>> > porting of git-rebase--interactive to C since the amount of extra code\n>>> > and support material added by this patch probably outweighs the amount\n>>> > of code a C version of git-rebase--interactive would need to count the\n>>> > lines itself.\n>>> >\n>>> > Stated differently, are the two or three instances of piping through\n>>> > 'wc' in git-rebase--interactive sufficient justification for\n>>> > introducing extra complexity into git-stripspace and its documentation\n>>> > and tests?\n>>>\n>>> Interesting thought. When somebody rewrites \"rebase -i\" in C,\n>>> nobody needs to count lines in \"stripspace\" output. The rewritten\n>>> \"rebase -i\" would internally run strbuf_stripspace() and the question\n>>> becomes what is the best way to let that code find out how many lines\n>>> the result contains.\n>>>\n>>> When viewed from that angle, I agree that \"stripspace --count\" does\n>>> not add anything to further the goal of helping \"rebase -i\" to move\n>>> to C. Adding strbuf_count_lines() that counts the number of lines\n>>> in the given strbuf (if there is no such helper yet; I didn't check),\n>>> though.\n>>\n>> I check before implementing this series and didn't find any helper. I\n>> also didn't find any other uses of line counting in the code.\n>\n> This shows that implementing \"git stripspace --count-lines\" could\n> indirectly help porting \"git rebase -i\" to C as you could implement\n> strbuf_count_lines() for the former and it could then be reused in the\n> latter.\n\nIn this project, where all user-facing functionality must be supported\nfor the life of the project, each new command, command-line option,\nconfiguration setting, and environment variable exacts additional\ncosts beyond the initial implementation cost. With this in mind, my\nquestion was also indirectly asking whether there was sufficient\njustification of the long-term cost of a --count-lines option. The\nargument that --count-lines would help test a proposed\nstrbuf_count_lines() likely does not outweigh that cost.\n\n>>> >> +test_expect_success '--count-lines with newline only' '\n>>> >> + printf \"0\\n\" >expect &&\n>>> >> + printf \"\\n\" | git stripspace --count-lines >actual &&\n>>> >> + test_cmp expect actual\n>>> >> +'\n>>> >\n>>> > What is the expected behavior when the input is an empty file, a file\n>>> > with content but no newline, a file with one or more lines but lacking\n>>> > a newline on the final line? Should these cases be tested, as well?\n>>>\n>>> Good point here, too. If we were to add strbuf_count_lines()\n>>> helper, whoever adds that function needs to take a possible\n>>> incomplete line at the end into account.\n>>\n>> Yes, makes more sense like this (even though it doesn't correspond to\n>> what 'wc -l' does).\n>\n> Tests for \"git stripspace --count-lines\" would test\n> strbuf_count_lines() which would also help when porting git rebase -i\n> to C.\n\nRather than saddling the project with the cost of a new user-facing,\nbut otherwise unneeded option, a more direct way to test the proposed\nstrbuf_count_lines() would be to add a test-strbuf program, akin to\ntest-config, test-string-list, etc. This has the added benefit of\nproviding a home for strbuf-based tests beyond line counting.\n"},{"id":"271947","messageId":"vpq4mhmablp.fsf@grenoble-inp.fr","threadId":"40573","inReplyTo":"CAPig+cR4wyumSfzXjptCfniuN0QC8TErL1X9LDPMsCD8wHP_kA@mail.gmail.com","subject":"Re: [PATCH v2 3/4] stripspace: Implement --count-lines option","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-10-19T19:42:58Z","receivedAt":"2015-10-19T19:42:58Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Eric Sunshine writes:\n\n> With this in mind, my\n> question was also indirectly asking whether there was sufficient\n> justification of the long-term cost of a --count-lines option. The\n> argument that --count-lines would help test a proposed\n> strbuf_count_lines() likely does not outweigh that cost.\n\nI agree. If we expect users to call --count-lines outside\nrebase-interactive.sh and our own tests, then the actual use should be\njustified in the commit message.\n\nIf not, then at least the --count-lines option should be hidden and not\ndocumented in the public doc. But I agree that introducing test-strbuf\nwould be even better.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"271961","messageId":"20151020084823.GP2468@distanz.ch","threadId":"40573","inReplyTo":"xmqq6125choi.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Tobias Klauser","fromEmail":"tklauser@distanz.ch","sentAt":"2015-10-20T08:48:23Z","receivedAt":"2015-10-20T08:48:23Z","isPatch":true,"sender":{"key":"tklauser@distanz.ch","avatar":"https://avatars.githubusercontent.com/u/539708?v=4"},"body":"On 2015-10-17 at 23:24:13 +0200, Junio C Hamano wrote:\n> Tobias Klauser writes:\n> \n> > On 2015-10-16 at 19:29:35 +0200, Junio C Hamano wrote:\n> >> Junio C Hamano writes:\n> >> \n> >> >> -\tif (mode == INVAL)\n> >> >> -\t\tusage(usage_msg);\n> >> >\n> >> > When given \"git stripspace -s blorg\", we used to set mode to INVAL\n> >> > and then showed the correct usage. But we no longer have a check\n> >> > that corresponds to the old INVAL thing, do we? Perhaps check argc\n> >> > to detect presence of an otherwise ignored non-option argument\n> >> > immediately after parse_options() returns?\n> >> \n> >> Perhaps like this.\n> >\n> > Thanks. I'll fold it into v3.\n> \n> Before starting v3, please fetch from me and check what is queued on\n> 'pu'. It may turn out that the fix-ups I did while queuing this\n> round is sufficient, in which case you can just say that instead ;-)\n\nNow that patches 3 and 4 will be dropped and no changes being necessary\nfor patches 1 and 2 (except for your changes that I see are already\nfolded into 'pu'), do you want me to submit a v3 of the series? Or is it\nenough if I ask you to drop patches 3 (stripspace: implement\n--count-lines option) and 4 (rebase -i: use \"stripspace --count-lines\"\nwhen counting todo items)?\n\nThanks\n"},{"id":"271974","messageId":"xmqq37x5a6e1.fsf@gitster.mtv.corp.google.com","threadId":"40573","inReplyTo":"20151020084823.GP2468@distanz.ch","subject":"Re: [PATCH v2 2/4] stripspace: Use parse-options for command-line parsing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-10-20T15:47:50Z","receivedAt":"2015-10-20T15:47:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tobias Klauser writes:\n\n> On 2015-10-17 at 23:24:13 +0200, Junio C Hamano wrote:\n>> Before starting v3, please fetch from me and check what is queued on\n>> 'pu'. It may turn out that the fix-ups I did while queuing this\n>> round is sufficient, in which case you can just say that instead ;-)\n>\n> Now that patches 3 and 4 will be dropped and no changes being necessary\n> for patches 1 and 2 (except for your changes that I see are already\n> folded into 'pu'), do you want me to submit a v3 of the series? Or is it\n> enough if I ask you to drop patches 3 (stripspace: implement\n> --count-lines option) and 4 (rebase -i: use \"stripspace --count-lines\"\n> when counting todo items)?\n\nYes, the latter is sufficient and preferrable (less work for both of\nus, without losing anything to help other people to review and\ndiscuss who may want to chime in).\n\nThanks.\n"}]}