git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2] editor.c: Libify launch_editor()

From
Stephan Beyer <s-beyer@gmx.net>
Date
Jul 18, 2008, 12:35 UTC
Message-ID
<1216384544-3856-1-git-send-email-s-beyer@gmx.net>
In-Reply-To
<alpine.DEB.1.00.0807181405510.3932@eeepc-johanness>

This patch removes exit()/die() calls and builtin-specific messages from launch_editor(), so that it can be used as a general libgit.a function to launch an editor.

Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
---
Hi,
Johannes Schindelin wrote:
Show 24 quoted lines
> On Fri, 18 Jul 2008, Stephan Beyer wrote:
> 
> > This patch removes exit()/die() calls and builtin-specific messages from 
> > launch_editor(), so that it can be used as a general libgit.a function 
> > to launch an editor.
> 
> Thanks.  Now we have to convince Junio that it is a good idea :-)
> 
> > diff --git a/builtin-commit.c b/builtin-commit.c
> > index ed3fe3f..64f69f3 100644
> > --- a/builtin-commit.c
> > +++ b/builtin-commit.c
> > @@ -647,7 +647,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
> >  		char index[PATH_MAX];
> >  		const char *env[2] = { index, NULL };
> >  		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
> > -		launch_editor(git_path(commit_editmsg), NULL, env);
> > +		if (launch_editor(git_path(commit_editmsg), NULL, env))
> > +			die("running editor failed.\n"
> > +			"Please supply the message using either -m or -F option.");
> 
> In the error case, run_editor() already said more than "running editor 
> failed.", right?  Maybe you just want to skip that line and keep the 
> second?

Well, I wanted to do it like that, but die_builtin() usually adds "fatal: ", so the message becomes like:

	error: There was a problem with the editor xxx.
	fatal: Please supply the message using either -m or -F option.
        ^^^^^^^^^^^^^
        That looks odd.
Btw, using the former patch, it is:
	error: There was a problem with the editor xxx.
	fatal: running editor failed.
	Please supply the message using either -m or -F option.
So I wonder if I should not die() at all, but do a
	fprintf(stderr, "Please supply the message using either -m or -F option.\n");
	exit(1);
which is done in *this* version of the patch. So that the message becomes:
	error: There was a problem with the editor 'xxx'.
	Please supply the message using either -m or -F option.
Show 15 quoted lines
> > diff --git a/editor.c b/editor.c
> > index 483b62d..5d7f5f9 100644
> > --- a/editor.c
> > +++ b/editor.c
> > @@ -17,9 +17,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
> >  	terminal = getenv("TERM");
> >  	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
> >  		fprintf(stderr,
> > -		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
> > -		"Please supply the message using either -m or -F option.\n");
> > -		exit(1);
> > +		"Terminal is dumb but no VISUAL nor EDITOR defined.\n");
> > +		return 1;
> 
> Why not "return error()"?
I was unsure here, too, but you're right.
Regards.
 builtin-commit.c |    6 +++++-
 builtin-tag.c    |    6 +++++-
 editor.c         |   24 ++++++++++++------------
 foo              |    1 +
 strbuf.h         |    2 +-
 5 files changed, 24 insertions(+), 15 deletions(-)
 create mode 100644 foo
diff --git a/builtin-commit.c b/builtin-commit.c
index ed3fe3f..23ec629 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -647,7 +647,11 @@ static int prepare_to_commit(const char *index_file, const char *prefix)
 		char index[PATH_MAX];
 		const char *env[2] = { index, NULL };
 		snprintf(index, sizeof(index), "GIT_INDEX_FILE=%s", index_file);
-		launch_editor(git_path(commit_editmsg), NULL, env);
+		if (launch_editor(git_path(commit_editmsg), NULL, env)) {
+			fprintf(stderr,
+			"Please supply the message using either -m or -F option.\n");
+			exit(1);
+		}
 	}
 
 	if (!no_verify &&
diff --git a/builtin-tag.c b/builtin-tag.c
index 219f51d..325b1b2 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -295,7 +295,11 @@ static void create_tag(const unsigned char *object, const char *tag,
 			write_or_die(fd, tag_template, strlen(tag_template));
 		close(fd);
 
-		launch_editor(path, buf, NULL);
+		if (launch_editor(path, buf, NULL)) {
+			fprintf(stderr,
+			"Please supply the message using either -m or -F option.\n");
+			exit(1);
+		}
 
 		unlink(path);
 		free(path);
diff --git a/editor.c b/editor.c
index 483b62d..eebc3e9 100644
--- a/editor.c
+++ b/editor.c
@@ -2,7 +2,7 @@
 #include "strbuf.h"
 #include "run-command.h"
 
-void launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
+int launch_editor(const char *path, struct strbuf *buffer, const char *const *env)
 {
 	const char *editor, *terminal;
 
@@ -15,12 +15,8 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
 		editor = getenv("EDITOR");
 
 	terminal = getenv("TERM");
-	if (!editor && (!terminal || !strcmp(terminal, "dumb"))) {
-		fprintf(stderr,
-		"Terminal is dumb but no VISUAL nor EDITOR defined.\n"
-		"Please supply the message using either -m or -F option.\n");
-		exit(1);
-	}
+	if (!editor && (!terminal || !strcmp(terminal, "dumb")))
+		return error("Terminal is dumb but no VISUAL nor EDITOR defined.");
 
 	if (!editor)
 		editor = "vi";
@@ -28,6 +24,7 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
 	if (strcmp(editor, ":")) {
 		size_t len = strlen(editor);
 		int i = 0;
+		int failed;
 		const char *args[6];
 		struct strbuf arg0;
 
@@ -43,14 +40,17 @@ void launch_editor(const char *path, struct strbuf *buffer, const char *const *e
 		args[i++] = path;
 		args[i] = NULL;
 
-		if (run_command_v_opt_cd_env(args, 0, NULL, env))
-			die("There was a problem with the editor %s.", editor);
+		failed = run_command_v_opt_cd_env(args, 0, NULL, env);
 		strbuf_release(&arg0);
+		if (failed)
+			return error("There was a problem with the editor '%s'.",
+					editor);
 	}
 
 	if (!buffer)
-		return;
+		return 0;
 	if (strbuf_read_file(buffer, path, 0) < 0)
-		die("could not read message file '%s': %s",
-		    path, strerror(errno));
+		return error("could not read file '%s': %s",
+				path, strerror(errno));
+	return 0;
 }
diff --git a/foo b/foo
new file mode 100644
index 0000000..8b13789
--- /dev/null
+++ b/foo
@@ -0,0 +1 @@
+
diff --git a/strbuf.h b/strbuf.h
index 0c6ffad..eba7ba4 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -123,6 +123,6 @@ extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);
 extern int strbuf_getline(struct strbuf *, FILE *, int);
 
 extern void stripspace(struct strbuf *buf, int skip_comments);
-extern void launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
+extern int launch_editor(const char *path, struct strbuf *buffer, const char *const *env);
 
 #endif /* STRBUF_H */

-- 
1.5.6.3.390.g7b30
Previous: Johannes SchindelinNext: Stephan Beyer
Message 6 of 17 in “Move launch_editor() from builtin-tag.c to editor.c”
  1. Move launch_editor() from builtin-tag.c to editor.cStephan Beyer, Jul 18, 2008
  2. Johannes SchindelinJul 18, 2008
  3. Stephan BeyerJul 18, 2008
  4. editor.c: Libify launch_editor()Stephan Beyer, Jul 18, 2008
  5. Johannes SchindelinJul 18, 2008
  6. editor.c: Libify launch_editor()Stephan Beyer, Jul 18, 2008
  7. Stephan BeyerJul 23, 2008
  8. Stephan BeyerJul 24, 2008
  9. Junio C HamanoJul 25, 2008
  10. Johannes SchindelinJul 25, 2008
  11. Stephan BeyerJul 25, 2008
  12. Johannes SchindelinJul 25, 2008
  13. 1/2 Move launch_editor() from builtin-tag.c to editor.cStephan Beyer, Jul 25, 2008
  14. 2/2 editor.c: Libify launch_editor()Stephan Beyer, Jul 25, 2008
  15. Stephan BeyerJul 25, 2008
  16. Johannes SchindelinJul 26, 2008
  17. Stephan BeyerJul 26, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.