{"thread":{"id":"14514","subject":"[PATCH] Move launch_editor() from builtin-tag.c to editor.c","startedAt":"2008-07-18T00:36:18Z","lastAt":"2008-07-26T03:14:51Z","messageCount":17,"participants":["Stephan Beyer","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"83776","messageId":"1216341378-15444-1-git-send-email-s-beyer@gmx.net","threadId":"14514","inReplyTo":null,"subject":"[PATCH] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-18T00:36:18Z","receivedAt":"2008-07-18T00:36:18Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"launch_editor() is declared in strbuf.h but defined in builtin-tag.c.\nThis patch moves launch_editor() into a new source file editor.c,\nbut keeps the declaration in strbuf.h.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\nHi,\n\nwhy editor.c and not strbuf.c?\nSee http://thread.gmane.org/gmane.comp.version-control.git/86636/focus=88939\n\nAnd I wonder why launch_editor is part of the strbuf API at all. ;)\n\nRegards,\n  Stephan\n\n Makefile      |    1 +\n builtin-tag.c |   53 -----------------------------------------------------\n editor.c      |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+), 53 deletions(-)\n create mode 100644 editor.c\n\ndiff --git a/Makefile b/Makefile\nindex 75c4ead..4bec4b3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -409,6 +409,7 @@ LIB_OBJS += diff-no-index.o\n LIB_OBJS += diff-lib.o\n LIB_OBJS += diff.o\n LIB_OBJS += dir.o\n+LIB_OBJS += editor.o\n LIB_OBJS += entry.o\n LIB_OBJS += environment.o\n LIB_OBJS += exec_cmd.o\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex c2cca6c..219f51d 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -23,59 +23,6 @@ static const char * const git_tag_usage[] = {\n \n static char signingkey[1000];\n \n-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\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\tsize_t len = strlen(editor);\n-\t\tint i = 0;\n-\t\tconst char *args[6];\n-\t\tstruct strbuf arg0;\n-\n-\t\tstrbuf_init(&arg0, 0);\n-\t\tif (strcspn(editor, \"$ \\t'\") != len) {\n-\t\t\t/* there are specials */\n-\t\t\tstrbuf_addf(&arg0, \"%s \\\"$@\\\"\", editor);\n-\t\t\targs[i++] = \"sh\";\n-\t\t\targs[i++] = \"-c\";\n-\t\t\targs[i++] = arg0.buf;\n-\t\t}\n-\t\targs[i++] = editor;\n-\t\targs[i++] = path;\n-\t\targs[i] = NULL;\n-\n-\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n-\t\tstrbuf_release(&arg0);\n-\t}\n-\n-\tif (!buffer)\n-\t\treturn;\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/editor.c b/editor.c\nnew file mode 100644\nindex 0000000..483b62d\n--- /dev/null\n+++ b/editor.c\n@@ -0,0 +1,56 @@\n+#include \"cache.h\"\n+#include \"strbuf.h\"\n+#include \"run-command.h\"\n+\n+void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\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\tsize_t len = strlen(editor);\n+\t\tint i = 0;\n+\t\tconst char *args[6];\n+\t\tstruct strbuf arg0;\n+\n+\t\tstrbuf_init(&arg0, 0);\n+\t\tif (strcspn(editor, \"$ \\t'\") != len) {\n+\t\t\t/* there are specials */\n+\t\t\tstrbuf_addf(&arg0, \"%s \\\"$@\\\"\", editor);\n+\t\t\targs[i++] = \"sh\";\n+\t\t\targs[i++] = \"-c\";\n+\t\t\targs[i++] = arg0.buf;\n+\t\t}\n+\t\targs[i++] = editor;\n+\t\targs[i++] = path;\n+\t\targs[i] = NULL;\n+\n+\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n+\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t\tstrbuf_release(&arg0);\n+\t}\n+\n+\tif (!buffer)\n+\t\treturn;\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.6.3.390.g7b30\n"},{"id":"83782","messageId":"alpine.DEB.1.00.0807180313200.2906@eeepc-johanness","threadId":"14514","inReplyTo":"1216341378-15444-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-18T01:14:42Z","receivedAt":"2008-07-18T01:14:42Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 18 Jul 2008, Stephan Beyer wrote:\n\n> launch_editor() is declared in strbuf.h but defined in builtin-tag.c. \n> This patch moves launch_editor() into a new source file editor.c, but \n> keeps the declaration in strbuf.h.\n\nSorry, but that has been tried before.  Junio pointed out that \nlaunch_editor() iss too die()-happy, and that the messages are too bound \nto the current callers.\n\nYou need to at least provide an add-on patch to fix both issues.\n\nCiao,\nDscho\n"},{"id":"83783","messageId":"20080718012604.GF8421@leksak.fem-net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807180313200.2906@eeepc-johanness","subject":"Re: [PATCH] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-18T01:26:04Z","receivedAt":"2008-07-18T01:26:04Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> Hi,\n> \n> On Fri, 18 Jul 2008, Stephan Beyer wrote:\n> \n> > launch_editor() is declared in strbuf.h but defined in builtin-tag.c. \n> > This patch moves launch_editor() into a new source file editor.c, but \n> > keeps the declaration in strbuf.h.\n> \n> Sorry, but that has been tried before.  Junio pointed out that \n> launch_editor() iss too die()-happy, and that the messages are too bound \n> to the current callers.\n\nAh, a short note: this is not a real \"libify\" patch. It is more an\n\"Oh, launch_editor is used for git-commit and git-tag, but it is only\n defined in builtin_tag.c, but declared (as libgit.a member) in strbuf.h,\n so let's move it\" patch without further changes.\n\nBut of course you and Junio are right.\n\n> You need to at least provide an add-on patch to fix both issues.\n\nTomorrow, I need sleep first :)\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"83867","messageId":"1216380408-21671-1-git-send-email-s-beyer@gmx.net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807180313200.2906@eeepc-johanness","subject":"[PATCH] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-18T11:26:48Z","receivedAt":"2008-07-18T11:26:48Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"This patch removes exit()/die() calls and builtin-specific messages\nfrom launch_editor(), so that it can be used as a general libgit.a\nfunction to launch an editor.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\nIs that ok? :)\n\n builtin-commit.c |    4 +++-\n builtin-tag.c    |    4 +++-\n editor.c         |   21 ++++++++++++---------\n strbuf.h         |    2 +-\n 4 files changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex ed3fe3f..64f69f3 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -647,7 +647,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\tchar index[PATH_MAX];\n \t\tconst char *env[2] = { index, NULL };\n \t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n+\t\tif (launch_editor(git_path(commit_editmsg), NULL, env))\n+\t\t\tdie(\"running editor failed.\\n\"\n+\t\t\t\"Please supply the message using either -m or -F option.\");\n \t}\n \n \tif (!no_verify &&\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 219f51d..3b7a25e 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -295,7 +295,9 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t\t\twrite_or_die(fd, tag_template, strlen(tag_template));\n \t\tclose(fd);\n \n-\t\tlaunch_editor(path, buf, NULL);\n+\t\tif (launch_editor(path, buf, NULL))\n+\t\t\tdie(\"running editor failed.\\n\"\n+\t\t\t\"Please supply the message using either -m or -F option.\");\n \n \t\tunlink(path);\n \t\tfree(path);\ndiff --git a/editor.c b/editor.c\nindex 483b62d..5d7f5f9 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -2,7 +2,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n \n-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n {\n \tconst char *editor, *terminal;\n \n@@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\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\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\");\n+\t\treturn 1;\n \t}\n \n \tif (!editor)\n@@ -28,6 +27,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \tif (strcmp(editor, \":\")) {\n \t\tsize_t len = strlen(editor);\n \t\tint i = 0;\n+\t\tint failed;\n \t\tconst char *args[6];\n \t\tstruct strbuf arg0;\n \n@@ -43,14 +43,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \t\targs[i++] = path;\n \t\targs[i] = NULL;\n \n-\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t\tfailed = run_command_v_opt_cd_env(args, 0, NULL, env);\n \t\tstrbuf_release(&arg0);\n+\t\tif (failed)\n+\t\t\treturn error(\"There was a problem with the editor %s.\",\n+\t\t\t\t\teditor);\n \t}\n \n \tif (!buffer)\n-\t\treturn;\n+\t\treturn 0;\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+\t\treturn error(\"could not read file '%s': %s\",\n+\t\t\t\tpath, strerror(errno));\n+\treturn 0;\n }\ndiff --git a/strbuf.h b/strbuf.h\nindex 0c6ffad..eba7ba4 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n \n #endif /* STRBUF_H */\n-- \n1.5.6.3.390.g7b30\n"},{"id":"83870","messageId":"alpine.DEB.1.00.0807181405510.3932@eeepc-johanness","threadId":"14514","inReplyTo":"1216380408-21671-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-18T12:07:57Z","receivedAt":"2008-07-18T12:07:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 18 Jul 2008, Stephan Beyer wrote:\n\n> This patch removes exit()/die() calls and builtin-specific messages from \n> launch_editor(), so that it can be used as a general libgit.a function \n> to launch an editor.\n\nThanks.  Now we have to convince Junio that it is a good idea :-)\n\n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index ed3fe3f..64f69f3 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -647,7 +647,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n>  \t\tchar index[PATH_MAX];\n>  \t\tconst char *env[2] = { index, NULL };\n>  \t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n> -\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n> +\t\tif (launch_editor(git_path(commit_editmsg), NULL, env))\n> +\t\t\tdie(\"running editor failed.\\n\"\n> +\t\t\t\"Please supply the message using either -m or -F option.\");\n\nIn the error case, run_editor() already said more than \"running editor \nfailed.\", right?  Maybe you just want to skip that line and keep the \nsecond?\n\n> diff --git a/editor.c b/editor.c\n> index 483b62d..5d7f5f9 100644\n> --- a/editor.c\n> +++ b/editor.c\n> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\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\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\");\n> +\t\treturn 1;\n\nWhy not \"return error()\"?\n\nRest looks obviously correct to me!\n\nThanks,\nDscho\n"},{"id":"83871","messageId":"1216384544-3856-1-git-send-email-s-beyer@gmx.net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807181405510.3932@eeepc-johanness","subject":"[PATCH v2] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-18T12:35:44Z","receivedAt":"2008-07-18T12:35:44Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"This patch removes exit()/die() calls and builtin-specific messages\nfrom launch_editor(), so that it can be used as a general libgit.a\nfunction to launch an editor.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\nHi,\n\nJohannes Schindelin wrote:\n> On Fri, 18 Jul 2008, Stephan Beyer wrote:\n> \n> > This patch removes exit()/die() calls and builtin-specific messages from \n> > launch_editor(), so that it can be used as a general libgit.a function \n> > to launch an editor.\n> \n> Thanks.  Now we have to convince Junio that it is a good idea :-)\n> \n> > diff --git a/builtin-commit.c b/builtin-commit.c\n> > index ed3fe3f..64f69f3 100644\n> > --- a/builtin-commit.c\n> > +++ b/builtin-commit.c\n> > @@ -647,7 +647,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n> >  \t\tchar index[PATH_MAX];\n> >  \t\tconst char *env[2] = { index, NULL };\n> >  \t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n> > -\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n> > +\t\tif (launch_editor(git_path(commit_editmsg), NULL, env))\n> > +\t\t\tdie(\"running editor failed.\\n\"\n> > +\t\t\t\"Please supply the message using either -m or -F option.\");\n> \n> In the error case, run_editor() already said more than \"running editor \n> failed.\", right?  Maybe you just want to skip that line and keep the \n> second?\n\nWell, I wanted to do it like that, but die_builtin() usually adds \"fatal: \",\nso the message becomes like:\n\n\terror: There was a problem with the editor xxx.\n\tfatal: Please supply the message using either -m or -F option.\n        ^^^^^^^^^^^^^\n        That looks odd.\n\nBtw, using the former patch, it is:\n\n\terror: There was a problem with the editor xxx.\n\tfatal: running editor failed.\n\tPlease supply the message using either -m or -F option.\n\n\nSo I wonder if I should not die() at all, but do a\n\tfprintf(stderr, \"Please supply the message using either -m or -F option.\\n\");\n\texit(1);\nwhich is done in *this* version of the patch. So that the message becomes:\n\n\terror: There was a problem with the editor 'xxx'.\n\tPlease supply the message using either -m or -F option.\n\n> > diff --git a/editor.c b/editor.c\n> > index 483b62d..5d7f5f9 100644\n> > --- a/editor.c\n> > +++ b/editor.c\n> > @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\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\t\"Terminal is dumb but no VISUAL nor EDITOR defined.\\n\");\n> > +\t\treturn 1;\n> \n> Why not \"return error()\"?\n\nI was unsure here, too, but you're right.\n\nRegards.\n\n builtin-commit.c |    6 +++++-\n builtin-tag.c    |    6 +++++-\n editor.c         |   24 ++++++++++++------------\n foo              |    1 +\n strbuf.h         |    2 +-\n 5 files changed, 24 insertions(+), 15 deletions(-)\n create mode 100644 foo\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex ed3fe3f..23ec629 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -647,7 +647,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\tchar index[PATH_MAX];\n \t\tconst char *env[2] = { index, NULL };\n \t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n+\t\tif (launch_editor(git_path(commit_editmsg), NULL, env)) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\"Please supply the message using either -m or -F option.\\n\");\n+\t\t\texit(1);\n+\t\t}\n \t}\n \n \tif (!no_verify &&\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 219f51d..325b1b2 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -295,7 +295,11 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t\t\twrite_or_die(fd, tag_template, strlen(tag_template));\n \t\tclose(fd);\n \n-\t\tlaunch_editor(path, buf, NULL);\n+\t\tif (launch_editor(path, buf, NULL)) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\"Please supply the message using either -m or -F option.\\n\");\n+\t\t\texit(1);\n+\t\t}\n \n \t\tunlink(path);\n \t\tfree(path);\ndiff --git a/editor.c b/editor.c\nindex 483b62d..eebc3e9 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -2,7 +2,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n \n-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n {\n \tconst char *editor, *terminal;\n \n@@ -15,12 +15,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\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+\tif (!editor && (!terminal || !strcmp(terminal, \"dumb\")))\n+\t\treturn error(\"Terminal is dumb but no VISUAL nor EDITOR defined.\");\n \n \tif (!editor)\n \t\teditor = \"vi\";\n@@ -28,6 +24,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \tif (strcmp(editor, \":\")) {\n \t\tsize_t len = strlen(editor);\n \t\tint i = 0;\n+\t\tint failed;\n \t\tconst char *args[6];\n \t\tstruct strbuf arg0;\n \n@@ -43,14 +40,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \t\targs[i++] = path;\n \t\targs[i] = NULL;\n \n-\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t\tfailed = run_command_v_opt_cd_env(args, 0, NULL, env);\n \t\tstrbuf_release(&arg0);\n+\t\tif (failed)\n+\t\t\treturn error(\"There was a problem with the editor '%s'.\",\n+\t\t\t\t\teditor);\n \t}\n \n \tif (!buffer)\n-\t\treturn;\n+\t\treturn 0;\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+\t\treturn error(\"could not read file '%s': %s\",\n+\t\t\t\tpath, strerror(errno));\n+\treturn 0;\n }\ndiff --git a/foo b/foo\nnew file mode 100644\nindex 0000000..8b13789\n--- /dev/null\n+++ b/foo\n@@ -0,0 +1 @@\n+\ndiff --git a/strbuf.h b/strbuf.h\nindex 0c6ffad..eba7ba4 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n \n #endif /* STRBUF_H */\n-- \n1.5.6.3.390.g7b30\n"},{"id":"84636","messageId":"20080723230923.GA12754@leksak.fem-net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807181405510.3932@eeepc-johanness","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-23T23:09:23Z","receivedAt":"2008-07-23T23:09:23Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> > This patch removes exit()/die() calls and builtin-specific messages from \n> > launch_editor(), so that it can be used as a general libgit.a function \n> > to launch an editor.\n> \n> Thanks.  Now we have to convince Junio that it is a good idea :-)\n\nWell, I've seen that *a lot* of lib code (15 functions, see below) is in \nthe builtins.\n\nCleaning that up seems to be good to have a real separation between\nlibgit and builtins, but I guess such a change would not find its way\ninto 1.6.0, would it?\n\nRegards,\n  Stephan\n\nPS: I've spontaneously decided to make a list:\n\ndefined-in\tfunc-name\t - used in builtin-\\1.c\n\nbuiltin-add.c:\n\tadd_files_to_cache()\t - add, checkout, commit\n\tinteractive_add()\t - add, commit\n\nbuiltin-archive.c:\n\tparse_pathspec_arg()\t - archive, uploard-archive\n\nbuiltin-init-db.c:\n\tinit_db()\t\t - init-db, clone\n\nbuiltin-ls-files.c:\n\toverlay_tree_on_cache()\t - ls-files, commit\n\treport_path_error()\t - ls-files, checkout, commit\n\nbuiltin-mailsplit.c:\n\tread_line_with_nul()\t - mailsplit, mailinfo\n\nbuiltin-merge-recursive.c:\n\twrite_tree_from_memory() - merge-recursive, checkout\n\nbuiltin-prune-packed.c:\n\tprune_packed_objects()\t - prune-packed, prune\n\nbuiltin-shortlog.c:\n\tshortlog_add_commit()\t - shortlog, log\n\tshortlog_init()\t\t - shortlog, log\n\tshortlog_output()\t - shortlog, log\n\tshortlog_init()\t\t - shortlog, log\n\nbuiltin-stripspace.c:\n\tstripspace()\t\t - stripspace, commit, tag\n\nAnd launch_editor() that is handled by the patch in this thread.\n\nThen there are functions that are non-static, but not declined in\nany .h file. A patch that makes them static follows in a separate mail.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"84662","messageId":"20080724014814.GA8730@leksak.fem-net","threadId":"14514","inReplyTo":"20080723230923.GA12754@leksak.fem-net","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-24T01:48:14Z","receivedAt":"2008-07-24T01:48:14Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Stephan Beyer wrote:\n> Well, I've seen that *a lot* of lib code (15 functions, see below) is in \n> the builtins.\n> \n> Cleaning that up seems to be good to have a real separation between\n> libgit and builtins, but I guess such a change would not find its way\n> into 1.6.0, would it?\n> \n> Regards,\n>   Stephan\n> \n> PS: I've spontaneously decided to make a list:\n> \n> defined-in\tfunc-name\t - used in builtin-\\1.c\n\nOf course my list was not complete.\n(My regex was too simple.)\n\nI add some:\n> builtin-add.c:\n> \tadd_files_to_cache()\t - add, checkout, commit\n> \tinteractive_add()\t - add, commit\n> \n> builtin-archive.c:\n> \tparse_pathspec_arg()\t - archive, upload-archive\n\n\tparse_treeish_arg()\t - archive, upload-archive\n\tparse_archive_args()\t - archive, upload-archive\n\nbuiltin-commit-tree.c:\n\tcommit_tree()\t\t - commit-tree, merge\n\nbuiltin-fetch-pack.c:\n\tfetch_pack()\t\t - also used in transport.c\n\n> builtin-init-db.c:\n> \tinit_db()\t\t - init-db, clone\n> \n> builtin-ls-files.c:\n> \toverlay_tree_on_cache()\t - ls-files, commit\n> \treport_path_error()\t - ls-files, checkout, commit\n\n\tpathspec_match()\t - ls-files, checkout, commit\n\n> builtin-mailsplit.c:\n> \tread_line_with_nul()\t - mailsplit, mailinfo\n> \n> builtin-merge-recursive.c:\n> \twrite_tree_from_memory() - merge-recursive, checkout\n\n\tmerge_trees()\t\t - merge-recursive, checkout\n\tmerge_recursive()\t - merge-recursive (used nowhere else)\n\n> builtin-prune-packed.c:\n> \tprune_packed_objects()\t - prune-packed, prune\n\nbuiltin-send-pack.c:\n\tsend_pack()\t\t - also used in transport.c\n\n> builtin-shortlog.c:\n> \tshortlog_add_commit()\t - shortlog, log\n> \tshortlog_init()\t\t - shortlog, log\n> \tshortlog_output()\t - shortlog, log\n> \tshortlog_init()\t\t - shortlog, log\n> \n> builtin-stripspace.c:\n> \tstripspace()\t\t - stripspace, commit, tag\n> \n> And launch_editor() that is handled by the patch in this thread.\n\n\nSo keep everything or is it worth splitting that somehow up?\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"84845","messageId":"7vvdyuuzq2.fsf@gitster.siamese.dyndns.org","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807181405510.3932@eeepc-johanness","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-07-25T08:36:53Z","receivedAt":"2008-07-25T08:36:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> Hi,\n>\n> On Fri, 18 Jul 2008, Stephan Beyer wrote:\n>\n>> This patch removes exit()/die() calls and builtin-specific messages from \n>> launch_editor(), so that it can be used as a general libgit.a function \n>> to launch an editor.\n>\n> Thanks.  Now we have to convince Junio that it is a good idea :-)\n\nEh, excuse me.\n\n>> diff --git a/editor.c b/editor.c\n>> index 483b62d..5d7f5f9 100644\n>> --- a/editor.c\n>> +++ b/editor.c\n>> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n> ...\n> Why not \"return error()\"?\n>\n> Rest looks obviously correct to me!\n\nThis is a patch to an existing file editor.c???\n"},{"id":"84857","messageId":"alpine.DEB.1.00.0807251226500.11976@eeepc-johanness","threadId":"14514","inReplyTo":"7vvdyuuzq2.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-25T10:28:03Z","receivedAt":"2008-07-25T10:28:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Jul 2008, Junio C Hamano wrote:\n\n> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> \n> > On Fri, 18 Jul 2008, Stephan Beyer wrote:\n> >\n> >> diff --git a/editor.c b/editor.c\n> >> index 483b62d..5d7f5f9 100644\n> >> --- a/editor.c\n> >> +++ b/editor.c\n> >> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n> > ...\n> > Why not \"return error()\"?\n> >\n> > Rest looks obviously correct to me!\n> \n> This is a patch to an existing file editor.c???\n\nYes, Stephan sent the two patches unrelatedly, even if they should have \nbeen marked \"1/2\" and \"2/2\".  I hope he does so now.\n\nCiao,\nDscho\n"},{"id":"84886","messageId":"20080725141517.GB18198@leksak.fem-net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807251226500.11976@eeepc-johanness","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-25T14:15:17Z","receivedAt":"2008-07-25T14:15:17Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> On Fri, 25 Jul 2008, Junio C Hamano wrote:\n> > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n> > \n> > > On Fri, 18 Jul 2008, Stephan Beyer wrote:\n> > >\n> > >> diff --git a/editor.c b/editor.c\n> > >> index 483b62d..5d7f5f9 100644\n> > >> --- a/editor.c\n> > >> +++ b/editor.c\n> > >> @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n> > > ...\n> > > Why not \"return error()\"?\n> > >\n> > > Rest looks obviously correct to me!\n> > \n> > This is a patch to an existing file editor.c???\n> \n> Yes, Stephan sent the two patches unrelatedly,\n\nYes, because when I've sent 1/2, I didn't knew that 2/2 will exist.\nSo I've just sent the second patch \"into this thread\" and hoped it would\nbe clear how it is meant. :)\n\nSo, to summarize:\n[PATCH 1/2] http://thread.gmane.org/gmane.comp.version-control.git/88940/focus=88940\n[PATCH 2/2] http://thread.gmane.org/gmane.comp.version-control.git/88940/focus=89035\n\nThey still apply cleanly to current master.\n\n> even if they should have been marked \"1/2\" and \"2/2\".\n> I hope he does so now.\n\nThis means I should send them again?\n\nRegards.\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"84887","messageId":"alpine.DEB.1.00.0807251636140.11976@eeepc-johanness","threadId":"14514","inReplyTo":"20080725141517.GB18198@leksak.fem-net","subject":"Re: [PATCH] editor.c: Libify launch_editor()","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-25T14:36:53Z","receivedAt":"2008-07-25T14:36:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Jul 2008, Stephan Beyer wrote:\n\n> Johannes Schindelin wrote:\n>\n> > even if they should have been marked \"1/2\" and \"2/2\". I hope he does \n> > so now.\n> \n> This means I should send them again?\n\nIf you want to be nice to the maintainer, yes.  If not, no.\n\nCiao,\nDscho\n"},{"id":"84899","messageId":"1217003322-10291-1-git-send-email-s-beyer@gmx.net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807251636140.11976@eeepc-johanness","subject":"[PATCH 1/2] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-25T16:28:41Z","receivedAt":"2008-07-25T16:28:41Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"launch_editor() is declared in strbuf.h but defined in builtin-tag.c.\nThis patch moves launch_editor() into a new source file editor.c,\nbut keeps the declaration in strbuf.h.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n\nJohannes Schindelin wrote:\n> > This means I should send them again?\n>\n> If you want to be nice to the maintainer, yes.  If not, no.\n\nI understand ;-)\n\nNo need to play the bad guy, so here's the patchset again.\n\nTo be kind to the maintainer, I've also run the test suite again,\nall tests passed except t4116*.sh, but this is not my fault.\nIt's the fault of 9a885fac.\n\nRegards,\n  Stephan\n\n Makefile      |    1 +\n builtin-tag.c |   53 -----------------------------------------------------\n editor.c      |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 57 insertions(+), 53 deletions(-)\n create mode 100644 editor.c\n\ndiff --git a/Makefile b/Makefile\nindex b01cf1c..df5b0d0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -410,6 +410,7 @@ LIB_OBJS += diff-no-index.o\n LIB_OBJS += diff-lib.o\n LIB_OBJS += diff.o\n LIB_OBJS += dir.o\n+LIB_OBJS += editor.o\n LIB_OBJS += entry.o\n LIB_OBJS += environment.o\n LIB_OBJS += exec_cmd.o\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex c2cca6c..219f51d 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -23,59 +23,6 @@ static const char * const git_tag_usage[] = {\n \n static char signingkey[1000];\n \n-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\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\tsize_t len = strlen(editor);\n-\t\tint i = 0;\n-\t\tconst char *args[6];\n-\t\tstruct strbuf arg0;\n-\n-\t\tstrbuf_init(&arg0, 0);\n-\t\tif (strcspn(editor, \"$ \\t'\") != len) {\n-\t\t\t/* there are specials */\n-\t\t\tstrbuf_addf(&arg0, \"%s \\\"$@\\\"\", editor);\n-\t\t\targs[i++] = \"sh\";\n-\t\t\targs[i++] = \"-c\";\n-\t\t\targs[i++] = arg0.buf;\n-\t\t}\n-\t\targs[i++] = editor;\n-\t\targs[i++] = path;\n-\t\targs[i] = NULL;\n-\n-\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n-\t\tstrbuf_release(&arg0);\n-\t}\n-\n-\tif (!buffer)\n-\t\treturn;\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/editor.c b/editor.c\nnew file mode 100644\nindex 0000000..483b62d\n--- /dev/null\n+++ b/editor.c\n@@ -0,0 +1,56 @@\n+#include \"cache.h\"\n+#include \"strbuf.h\"\n+#include \"run-command.h\"\n+\n+void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\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\tsize_t len = strlen(editor);\n+\t\tint i = 0;\n+\t\tconst char *args[6];\n+\t\tstruct strbuf arg0;\n+\n+\t\tstrbuf_init(&arg0, 0);\n+\t\tif (strcspn(editor, \"$ \\t'\") != len) {\n+\t\t\t/* there are specials */\n+\t\t\tstrbuf_addf(&arg0, \"%s \\\"$@\\\"\", editor);\n+\t\t\targs[i++] = \"sh\";\n+\t\t\targs[i++] = \"-c\";\n+\t\t\targs[i++] = arg0.buf;\n+\t\t}\n+\t\targs[i++] = editor;\n+\t\targs[i++] = path;\n+\t\targs[i] = NULL;\n+\n+\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n+\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t\tstrbuf_release(&arg0);\n+\t}\n+\n+\tif (!buffer)\n+\t\treturn;\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.6.0.rc0.102.ga1791\n"},{"id":"84898","messageId":"1217003322-10291-2-git-send-email-s-beyer@gmx.net","threadId":"14514","inReplyTo":"1217003322-10291-1-git-send-email-s-beyer@gmx.net","subject":"[PATCH 2/2] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-25T16:28:42Z","receivedAt":"2008-07-25T16:28:42Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"This patch removes exit()/die() calls and builtin-specific messages\nfrom launch_editor(), so that it can be used as a general libgit.a\nfunction to launch an editor.\n\nSigned-off-by: Stephan Beyer <s-beyer@gmx.net>\n---\n builtin-commit.c |    6 +++++-\n builtin-tag.c    |    6 +++++-\n editor.c         |   24 ++++++++++++------------\n foo              |    1 +\n strbuf.h         |    2 +-\n 5 files changed, 24 insertions(+), 15 deletions(-)\n create mode 100644 foo\n\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex 6a9dc0e..9a11ca0 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -646,7 +646,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)\n \t\tchar index[PATH_MAX];\n \t\tconst char *env[2] = { index, NULL };\n \t\tsnprintf(index, sizeof(index), \"GIT_INDEX_FILE=%s\", index_file);\n-\t\tlaunch_editor(git_path(commit_editmsg), NULL, env);\n+\t\tif (launch_editor(git_path(commit_editmsg), NULL, env)) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\"Please supply the message using either -m or -F option.\\n\");\n+\t\t\texit(1);\n+\t\t}\n \t}\n \n \tif (!no_verify &&\ndiff --git a/builtin-tag.c b/builtin-tag.c\nindex 219f51d..325b1b2 100644\n--- a/builtin-tag.c\n+++ b/builtin-tag.c\n@@ -295,7 +295,11 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t\t\twrite_or_die(fd, tag_template, strlen(tag_template));\n \t\tclose(fd);\n \n-\t\tlaunch_editor(path, buf, NULL);\n+\t\tif (launch_editor(path, buf, NULL)) {\n+\t\t\tfprintf(stderr,\n+\t\t\t\"Please supply the message using either -m or -F option.\\n\");\n+\t\t\texit(1);\n+\t\t}\n \n \t\tunlink(path);\n \t\tfree(path);\ndiff --git a/editor.c b/editor.c\nindex 483b62d..eebc3e9 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -2,7 +2,7 @@\n #include \"strbuf.h\"\n #include \"run-command.h\"\n \n-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)\n {\n \tconst char *editor, *terminal;\n \n@@ -15,12 +15,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\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+\tif (!editor && (!terminal || !strcmp(terminal, \"dumb\")))\n+\t\treturn error(\"Terminal is dumb but no VISUAL nor EDITOR defined.\");\n \n \tif (!editor)\n \t\teditor = \"vi\";\n@@ -28,6 +24,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \tif (strcmp(editor, \":\")) {\n \t\tsize_t len = strlen(editor);\n \t\tint i = 0;\n+\t\tint failed;\n \t\tconst char *args[6];\n \t\tstruct strbuf arg0;\n \n@@ -43,14 +40,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e\n \t\targs[i++] = path;\n \t\targs[i] = NULL;\n \n-\t\tif (run_command_v_opt_cd_env(args, 0, NULL, env))\n-\t\t\tdie(\"There was a problem with the editor %s.\", editor);\n+\t\tfailed = run_command_v_opt_cd_env(args, 0, NULL, env);\n \t\tstrbuf_release(&arg0);\n+\t\tif (failed)\n+\t\t\treturn error(\"There was a problem with the editor '%s'.\",\n+\t\t\t\t\teditor);\n \t}\n \n \tif (!buffer)\n-\t\treturn;\n+\t\treturn 0;\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+\t\treturn error(\"could not read file '%s': %s\",\n+\t\t\t\tpath, strerror(errno));\n+\treturn 0;\n }\ndiff --git a/foo b/foo\nnew file mode 100644\nindex 0000000..8b13789\n--- /dev/null\n+++ b/foo\n@@ -0,0 +1 @@\n+\ndiff --git a/strbuf.h b/strbuf.h\nindex 0c6ffad..eba7ba4 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);\n \n #endif /* STRBUF_H */\n-- \n1.6.0.rc0.102.ga1791\n"},{"id":"84909","messageId":"20080725171612.GF27172@leksak.fem-net","threadId":"14514","inReplyTo":"1217003322-10291-2-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH 2/2] editor.c: Libify launch_editor()","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-25T17:16:12Z","receivedAt":"2008-07-25T17:16:12Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nStephan Beyer wrote:\n> This patch removes exit()/die() calls and builtin-specific messages\n> from launch_editor(), so that it can be used as a general libgit.a\n> function to launch an editor.\n> \n> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>\n> ---\n>  builtin-commit.c |    6 +++++-\n>  builtin-tag.c    |    6 +++++-\n>  editor.c         |   24 ++++++++++++------------\n>  foo              |    1 +\n\nOuch! Please exclude \"foo\"...\nSorry, for this maintainer-unfriendly patch again. *embarrassed*\n\nStephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"},{"id":"84993","messageId":"alpine.DEB.1.00.0807260456360.26810@eeepc-johanness","threadId":"14514","inReplyTo":"1217003322-10291-1-git-send-email-s-beyer@gmx.net","subject":"Re: [PATCH 1/2] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-07-26T03:00:44Z","receivedAt":"2008-07-26T03:00:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 25 Jul 2008, Stephan Beyer wrote:\n\n> To be kind to the maintainer, I've also run the test suite again, all \n> tests passed except t4116*.sh, but this is not my fault. It's the fault \n> of 9a885fac.\n\nYou do understand that you cost everybody, who actually cared to take a \nlook for herself, a few minutes?\n\nJust to see that the change you referenced (but did not describe at all) \nis \"tar -> $TAR\".\n\nAnd now, everybody who cared will be just puzzled.  In the best case, he \nwill reply to you that your hint left to be wished for.  I do not have to \ndescribe the worst case, do I?\n\nCiao,\nDscho \"who thinks that so many mails would be better if the posters would \nread the mails themselves and try to imagine how readers would perceive \nthem\"\n"},{"id":"84996","messageId":"20080726031451.GG13539@leksak.fem-net","threadId":"14514","inReplyTo":"alpine.DEB.1.00.0807260456360.26810@eeepc-johanness","subject":"Re: [PATCH 1/2] Move launch_editor() from builtin-tag.c to editor.c","fromName":"Stephan Beyer","fromEmail":"s-beyer@gmx.net","sentAt":"2008-07-26T03:14:51Z","receivedAt":"2008-07-26T03:14:51Z","isPatch":true,"sender":{"key":"s-beyer@gmx.net","avatar":"https://avatars.githubusercontent.com/u/143889?v=4"},"body":"Hi,\n\nJohannes Schindelin wrote:\n> On Fri, 25 Jul 2008, Stephan Beyer wrote:\n> \n> > To be kind to the maintainer, I've also run the test suite again, all \n> > tests passed except t4116*.sh, but this is not my fault. It's the fault \n> > of 9a885fac.\n> \n> You do understand that you cost everybody, who actually cared to take a \n> look for herself, a few minutes?\n\nYes, now I see that I forgot to mention the subject.\n\nI'm sorry.\n\n> Dscho \"who thinks that so many mails would be better if the posters would \n> read the mails themselves and try to imagine how readers would perceive \n> them\"\n\nYou're right.\n\nIn my case I perhaps should've *first* looked if I could fix the TAR issue\nand then first send the fix for the TAR stuff and then remove the text from\nhere. But I noticed the failure, wrote about it here, sent this mail, looked\nfor the reason of the error, ... the wrong order, as it seems.\n\nI try to improve.\n\nRegards,\n  Stephan\n\n-- \nStephan Beyer <s-beyer@gmx.net>, PGP 0x6EDDD207FCC5040F\n"}]}