{"thread":{"id":"10809","subject":"[PATCH] Move stripspace() and launch_editor() to strbuf.c","startedAt":"2007-11-11T17:29:12Z","lastAt":"2007-11-11T22:52:26Z","messageCount":4,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"59340","messageId":"Pine.LNX.4.64.0711111728380.4362@racer.site","threadId":"10809","inReplyTo":null,"subject":"[PATCH] Move stripspace() and launch_editor() to strbuf.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T17:29:12Z","receivedAt":"2007-11-11T17:29:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nThese functions are already declared in strbuf.h, so it is only\nlogical to move their implementations to the corresponding file.\nParticularly, since strbuf.h is in LIB_H, but both functions\nwere missing from libgit.a.\n\nSigned-off-by: Johannes Schindelin \n---\n builtin-stripspace.c | 67 --------------------------------\n builtin-tag.c | 35 -----------------\n strbuf.c | 103 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 103 insertions(+), 102 deletions(-)\n\ndiff --git a/builtin-stripspace.c b/builtin-stripspace.c\nindex c0b2130..5de5a3f 100644\n--- a/builtin-stripspace.c\n+++ b/builtin-stripspace.c\n@@ -1,73 +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- * Note that the buffer will not be NUL-terminated.\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 \"#\".\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] == '#') {\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-\n int cmd_stripspace(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf;\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 566b9d1..c70564b 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -17,41 +17,6 @@ static const char builtin_tag_usage[] =\n \n static char signingkey[1000];\n \n-void launch_editor(const char *path, struct strbuf *buffer)\n-{\n-\tconst char *editor, *terminal;\n-\n-\teditor = getenv(\"GIT_EDITOR\");\n-\tif (!editor && editor_program)\n-\t\teditor = editor_program;\n-\tif (!editor)\n-\t\teditor = getenv(\"VISUAL\");\n-\tif (!editor)\n-\t\teditor = getenv(\"EDITOR\");\n-\n-\tterminal = getenv(\"TERM\");\n-\tif (!editor && (!terminal || !strcmp(terminal, \"dumb\"))) {\n-\t\tfprintf(stderr,\n-\t\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\"\n-\t\t\"Please supply the message using either -m or -F option.\\n\");\n-\t\texit(1);\n-\t}\n-\n-\tif (!editor)\n-\t\teditor = \"vi\";\n-\n-\tif (strcmp(editor, \":\")) {\n-\t\tconst char *args[] = { editor, path, NULL };\n-\n-\t\tif (run_command_v_opt(args, 0))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n-\t}\n-\n-\tif (strbuf_read_file(buffer, path, 0) < 0)\n-\t\tdie(\"could not read message file '%s': %s\",\n-\t\t path, strerror(errno));\n-}\n-\n struct tag_filter {\n \tconst char *pattern;\n \tint lines;\ndiff --git a/strbuf.c b/strbuf.c\nindex 536b432..6e99358 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"run-command.h\"\n \n /*\n * Used as the default ->buf value, so that people can always assume\n@@ -224,3 +225,105 @@ int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n \n \treturn 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+ * Note that the buffer will not be NUL-terminated.\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 \"#\".\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] == '#') {\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+\n+void launch_editor(const char *path, struct strbuf *buffer)\n+{\n+\tconst char *editor, *terminal;\n+\n+\teditor = getenv(\"GIT_EDITOR\");\n+\tif (!editor && editor_program)\n+\t\teditor = editor_program;\n+\tif (!editor)\n+\t\teditor = getenv(\"VISUAL\");\n+\tif (!editor)\n+\t\teditor = getenv(\"EDITOR\");\n+\n+\tterminal = getenv(\"TERM\");\n+\tif (!editor && (!terminal || !strcmp(terminal, \"dumb\"))) {\n+\t\tfprintf(stderr,\n+\t\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\"\n+\t\t\"Please supply the message using either -m or -F option.\\n\");\n+\t\texit(1);\n+\t}\n+\n+\tif (!editor)\n+\t\teditor = \"vi\";\n+\n+\tif (strcmp(editor, \":\")) {\n+\t\tconst char *args[] = { editor, path, NULL };\n+\n+\t\tif (run_command_v_opt(args, 0))\n+\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t}\n+\n+\tif (strbuf_read_file(buffer, path, 0) < 0)\n+\t\tdie(\"could not read message file '%s': %s\",\n+\t\t path, strerror(errno));\n+}\n-- \n1.5.3.5.1693.g26ed\n"},{"id":"59385","messageId":"7vtznsbgcc.fsf@gitster.siamese.dyndns.org","threadId":"10809","inReplyTo":"Pine.LNX.4.64.0711111728380.4362@racer.site","subject":"Re: [PATCH] Move stripspace() and launch_editor() to strbuf.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-11T21:52:03Z","receivedAt":"2007-11-11T21:52:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> These functions are already declared in strbuf.h, so it is only\n> logical to move their implementations to the corresponding file.\n> Particularly, since strbuf.h is in LIB_H, but both functions\n> were missing from libgit.a.\n\nI think this makes sense for stripspace(), but I have trouble\nwith launch_editor().\n\nI do not object to have a function in strbuf API that takes a\nbuffer, allows the end user to interactively edit its content\nand returns the updated content. The function was perfectly\nfine as a special purpose helper for git-commit and git-tag, but\nif you look at the current launch_editor(), it is not suitable\nas a generic strbuf library function:\n\n * \"Launch\" feels as if we are initiating an async operation and\n returning from the function without waiting for its\n completion, but this is not \"launch\" but \"launch, wait and\n return the resulting string\". Probably this should be called\n edit_buffer() or something like that.\n\n * Instead of dying, it should return exit code and have the\n caller choose to die or take any alternative action. The\n library function definitely should not say \"if you are in an\n environment where we cannot let you interactively edit, use\n -F or -m option\".\n"},{"id":"59391","messageId":"Pine.LNX.4.64.0711112227420.4362@racer.site","threadId":"10809","inReplyTo":"7vtznsbgcc.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Move stripspace() and launch_editor() to strbuf.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-11T22:29:34Z","receivedAt":"2007-11-11T22:29:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 11 Nov 2007, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > These functions are already declared in strbuf.h, so it is only\n> > logical to move their implementations to the corresponding file.\n> > Particularly, since strbuf.h is in LIB_H, but both functions\n> > were missing from libgit.a.\n> \n> I think this makes sense for stripspace(), but I have trouble\n> with launch_editor().\n> \n> I do not object to have a function in strbuf API that takes a\n> buffer, allows the end user to interactively edit its content\n> and returns the updated content. The function was perfectly\n> fine as a special purpose helper for git-commit and git-tag, but\n> if you look at the current launch_editor(), it is not suitable\n> as a generic strbuf library function:\n> \n> * \"Launch\" feels as if we are initiating an async operation and\n> returning from the function without waiting for its\n> completion, but this is not \"launch\" but \"launch, wait and\n> return the resulting string\". Probably this should be called\n> edit_buffer() or something like that.\n> \n> * Instead of dying, it should return exit code and have the\n> caller choose to die or take any alternative action. The\n> library function definitely should not say \"if you are in an\n> environment where we cannot let you interactively edit, use\n> -F or -m option\".\n\nAll valid points. Will add to my TODO list and repost when done.\n\nMaybe call_editor() instead of edit_buffer()? Since we technically read \nthe file specified by \"path\" into the buffer \"buffer\", after having called \nthe editor.\n\nCiao,\nDscho\n"},{"id":"59399","messageId":"7v6408bdjp.fsf@gitster.siamese.dyndns.org","threadId":"10809","inReplyTo":"Pine.LNX.4.64.0711112227420.4362@racer.site","subject":"Re: [PATCH] Move stripspace() and launch_editor() to strbuf.c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-11T22:52:26Z","receivedAt":"2007-11-11T22:52:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Maybe call_editor() instead of edit_buffer()? Since we technically read \n> the file specified by \"path\" into the buffer \"buffer\", after having called \n> the editor.\n\nCalling $EDITOR to edit the buffer is an implementation detail\nof letting the user edit the buffer. I think the function name\nshould express what it does more than how it does it.\n\nMy suggested name does not convey the \"letting the user\" part,\nthough.\n"}]}