{"thread":{"id":"47344","subject":"[PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","startedAt":"2017-11-29T14:38:28Z","lastAt":"2017-12-04T22:09:15Z","messageCount":31,"participants":["lars.schneider@autodesk.com","Thomas Adam","Lars Schneider","Andreas Schwab","Jeff King","Kaartic Sivaraam","Junio C Hamano"],"isPatch":true,"patchVersion":4,"patchTotal":2},"messages":[{"id":"333780","messageId":"20171129143752.60553-1-lars.schneider@autodesk.com","threadId":"47344","inReplyTo":null,"subject":"[PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2017-11-29T14:37:50Z","receivedAt":"2017-11-29T14:38:28Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nHi,\n\nI simplified the code and enabled the \"Git is waiting for editor input\"\nmessage only for \"intelligent\" terminals. The rational is, that today's\nnovice Git users are likely to have an \"intelligent\" terminal. People\nworking with \"dumb\" terminals have their reasons and are likely not the\ntarget audience for this hint.\n\nIn addition, I added \"advice.waitingForEditor\" as an easy way to disable\nthe hint for experienced users.\n\nI refactored the detection of dumb terminals in a preparation commit.\n\nThanks,\nLars\n\n\nRFC: https://public-inbox.org/git/274B4850-2EB7-4BFA-A42C-25A573254969@gmail.com/\n v1: https://public-inbox.org/git/xmqqr2syvjxb.fsf@gitster.mtv.corp.google.com/\n v2: https://public-inbox.org/git/20171117135109.18071-1-lars.schneider@autodesk.com/\n v3: https://public-inbox.org/git/20171127134716.69471-1-lars.schneider@autodesk.com/\n\nBase Ref: master\nWeb-Diff: https://github.com/larsxschneider/git/commit/9639e931bf\nCheckout: git fetch https://github.com/larsxschneider/git editor-v4 && git checkout 9639e931bf\n\n\n### Interdiff (v3..v4):\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 671fcbaa0f..de64201e82 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -354,6 +354,9 @@ advice.*::\n \tignoredHook::\n \t\tAdvice shown if an hook is ignored because the hook is not\n \t\tset as executable.\n+\twaitingForEditor::\n+\t\tPrint a message to the terminal whenever Git is waiting for\n+\t\teditor input from the user.\n --\n\n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex c6169bcb52..406efc183b 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -18,6 +18,7 @@ int advice_object_name_warning = 1;\n int advice_rm_hints = 1;\n int advice_add_embedded_repo = 1;\n int advice_ignored_hook = 1;\n+int advice_waiting_for_editor = 1;\n\n static struct {\n \tconst char *name;\n@@ -40,6 +41,7 @@ static struct {\n \t{ \"rmhints\", &advice_rm_hints },\n \t{ \"addembeddedrepo\", &advice_add_embedded_repo },\n \t{ \"ignoredhook\", &advice_ignored_hook },\n+\t{ \"waitingforeditor\", &advice_waiting_for_editor },\n\n \t/* make this an alias for backward compatibility */\n \t{ \"pushnonfastforward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f525d6f89c..70568fa792 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -20,6 +20,7 @@ extern int advice_object_name_warning;\n extern int advice_rm_hints;\n extern int advice_add_embedded_repo;\n extern int advice_ignored_hook;\n+extern int advice_waiting_for_editor;\n\n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/cache.h b/cache.h\nindex 89f5d24579..3842fc097c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void);\n extern const char *ident_default_email(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n+extern int is_terminal_dumb(void);\n extern int git_ident_config(const char *, const char *, void *);\n extern void reset_ident_date(void);\n\ndiff --git a/color.c b/color.c\nindex 9a9261ac16..d48dd947c9 100644\n--- a/color.c\n+++ b/color.c\n@@ -329,8 +329,7 @@ static int check_auto_color(void)\n \tif (color_stdout_is_tty < 0)\n \t\tcolor_stdout_is_tty = isatty(1);\n \tif (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {\n-\t\tchar *term = getenv(\"TERM\");\n-\t\tif (term && strcmp(term, \"dumb\"))\n+\t\tif (!is_terminal_dumb())\n \t\t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/editor.c b/editor.c\nindex 4251ae9d7a..cdad4f74ec 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -7,11 +7,16 @@\n #define DEFAULT_EDITOR \"vi\"\n #endif\n\n+int is_terminal_dumb(void)\n+{\n+\tconst char *terminal = getenv(\"TERM\");\n+\treturn !terminal || !strcmp(terminal, \"dumb\");\n+}\n+\n const char *git_editor(void)\n {\n \tconst char *editor = getenv(\"GIT_EDITOR\");\n-\tconst char *terminal = getenv(\"TERM\");\n-\tint terminal_is_dumb = !terminal || !strcmp(terminal, \"dumb\");\n+\tint terminal_is_dumb = is_terminal_dumb();\n\n \tif (!editor && editor_program)\n \t\teditor = editor_program;\n@@ -40,33 +45,11 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tconst char *args[] = { editor, real_path(path), NULL };\n \t\tstruct child_process p = CHILD_PROCESS_INIT;\n \t\tint ret, sig;\n-\t\tstatic const char *close_notice = NULL;\n-\n-\t\tif (isatty(2) && !close_notice) {\n-\t\t\tchar *term = getenv(\"TERM\");\n-\n-\t\t\tif (term && strcmp(term, \"dumb\"))\n-\t\t\t\t/*\n-\t\t\t\t * go back to the beginning and erase the\n-\t\t\t\t * entire line if the terminal is capable\n-\t\t\t\t * to do so, to avoid wasting the vertical\n-\t\t\t\t * space.\n-\t\t\t\t */\n-\t\t\t\tclose_notice = \"\\r\\033[K\";\n-\t\t\telse if (term && strstr(term, \"emacsclient\"))\n-\t\t\t\t/*\n-\t\t\t\t * `emacsclient` (or `emacsclientw` on Windows) already prints\n-\t\t\t\t * (\"Waiting for Emacs...\") if a file is opened for editing.\n-\t\t\t\t * Therefore, we don't need to print the editor launch info.\n-\t\t\t\t */\n-\t\t\t\t;\n-\t\t\telse\n-\t\t\t\t/* otherwise, complete and waste the line */\n-\t\t\t\tclose_notice = _(\"done.\\n\");\n-\t\t}\n+\t\tint print_waiting_for_editor = advice_waiting_for_editor &&\n+\t\t\tisatty(2) && !is_terminal_dumb();\n\n-\t\tif (close_notice) {\n-\t\t\tfprintf(stderr, _(\"Launched editor. Waiting for your input... \"));\n+\t\tif (print_waiting_for_editor) {\n+\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n \t\t\tfflush(stderr);\n \t\t}\n\n@@ -82,14 +65,19 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tsig = ret - 128;\n \t\tsigchain_pop(SIGINT);\n \t\tsigchain_pop(SIGQUIT);\n-\n \t\tif (sig == SIGINT || sig == SIGQUIT)\n \t\t\traise(sig);\n \t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n-\t\tif (close_notice)\n-\t\t\tfputs(close_notice, stderr);\n+\n+\t\tif (print_waiting_for_editor)\n+\t\t\t/*\n+\t\t\t * go back to the beginning and erase the\n+\t\t\t * entire line to avoid wasting the vertical\n+\t\t\t * space.\n+\t\t\t */\n+\t\t\tfputs(\"\\r\\033[K\", stderr);\n \t}\n\n \tif (!buffer)\ndiff --git a/sideband.c b/sideband.c\nindex 1e4d684d6c..6d7f943e43 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -20,13 +20,12 @@\n\n int recv_sideband(const char *me, int in_stream, int out)\n {\n-\tconst char *term, *suffix;\n+\tconst char *suffix;\n \tchar buf[LARGE_PACKET_MAX + 1];\n \tstruct strbuf outbuf = STRBUF_INIT;\n \tint retval = 0;\n\n-\tterm = getenv(\"TERM\");\n-\tif (isatty(2) && term && strcmp(term, \"dumb\"))\n+\tif (isatty(2) && !is_terminal_dumb())\n \t\tsuffix = ANSI_SUFFIX;\n \telse\n \t\tsuffix = DUMB_SUFFIX;\n\n\n### Patches\n\nLars Schneider (2):\n refactor \"dumb\" terminal determination\n launch_editor(): indicate that Git waits for user input\n\n Documentation/config.txt | 3 +++\n advice.c | 2 ++\n advice.h | 1 +\n cache.h | 1 +\n color.c | 3 +--\n editor.c | 24 ++++++++++++++++++++++--\n sideband.c | 5 ++---\n 7 files changed, 32 insertions(+), 7 deletions(-)\n\n\nbase-commit: 89ea799ffcc5c8a0547d3c9075eb979256ee95b8\n--\n2.15.0\n\n"},{"id":"333781","messageId":"20171129143752.60553-2-lars.schneider@autodesk.com","threadId":"47344","inReplyTo":"20171129143752.60553-1-lars.schneider@autodesk.com","subject":"[PATCH v4 1/2] refactor \"dumb\" terminal determination","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2017-11-29T14:37:51Z","receivedAt":"2017-11-29T14:38:30Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nMove the code to detect \"dumb\" terminals into a single location. This\navoids duplicating the terminal detection code yet again in a subsequent\ncommit.\n\nSigned-off-by: Lars Schneider \n---\n cache.h | 1 +\n color.c | 3 +--\n editor.c | 9 +++++++--\n sideband.c | 5 ++---\n 4 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 89f5d24579..3842fc097c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1469,6 +1469,7 @@ extern const char *ident_default_name(void);\n extern const char *ident_default_email(void);\n extern const char *git_editor(void);\n extern const char *git_pager(int stdout_is_tty);\n+extern int is_terminal_dumb(void);\n extern int git_ident_config(const char *, const char *, void *);\n extern void reset_ident_date(void);\n \ndiff --git a/color.c b/color.c\nindex 9a9261ac16..d48dd947c9 100644\n--- a/color.c\n+++ b/color.c\n@@ -329,8 +329,7 @@ static int check_auto_color(void)\n \tif (color_stdout_is_tty < 0)\n \t\tcolor_stdout_is_tty = isatty(1);\n \tif (color_stdout_is_tty || (pager_in_use() && pager_use_color)) {\n-\t\tchar *term = getenv(\"TERM\");\n-\t\tif (term && strcmp(term, \"dumb\"))\n+\t\tif (!is_terminal_dumb())\n \t\t\treturn 1;\n \t}\n \treturn 0;\ndiff --git a/editor.c b/editor.c\nindex 7519edecdc..c65ea698eb 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -7,11 +7,16 @@\n #define DEFAULT_EDITOR \"vi\"\n #endif\n \n+int is_terminal_dumb(void)\n+{\n+\tconst char *terminal = getenv(\"TERM\");\n+\treturn !terminal || !strcmp(terminal, \"dumb\");\n+}\n+\n const char *git_editor(void)\n {\n \tconst char *editor = getenv(\"GIT_EDITOR\");\n-\tconst char *terminal = getenv(\"TERM\");\n-\tint terminal_is_dumb = !terminal || !strcmp(terminal, \"dumb\");\n+\tint terminal_is_dumb = is_terminal_dumb();\n \n \tif (!editor && editor_program)\n \t\teditor = editor_program;\ndiff --git a/sideband.c b/sideband.c\nindex 1e4d684d6c..6d7f943e43 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -20,13 +20,12 @@\n \n int recv_sideband(const char *me, int in_stream, int out)\n {\n-\tconst char *term, *suffix;\n+\tconst char *suffix;\n \tchar buf[LARGE_PACKET_MAX + 1];\n \tstruct strbuf outbuf = STRBUF_INIT;\n \tint retval = 0;\n \n-\tterm = getenv(\"TERM\");\n-\tif (isatty(2) && term && strcmp(term, \"dumb\"))\n+\tif (isatty(2) && !is_terminal_dumb())\n \t\tsuffix = ANSI_SUFFIX;\n \telse\n \t\tsuffix = DUMB_SUFFIX;\n-- \n2.15.0\n\n"},{"id":"333782","messageId":"20171129143752.60553-3-lars.schneider@autodesk.com","threadId":"47344","inReplyTo":"20171129143752.60553-1-lars.schneider@autodesk.com","subject":"[PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"","fromEmail":"lars.schneider@autodesk.com","sentAt":"2017-11-29T14:37:52Z","receivedAt":"2017-11-29T14:38:33Z","isPatch":true,"sender":{"key":"lars.schneider@autodesk.com","avatar":"https://gravatar.com/avatar/524188df3a38b02692619777bb90d9cc735db1ad75f5304e0bf09f8efd299775?d=mp&s=160"},"body":"From: Lars Schneider \n\nWhen a graphical GIT_EDITOR is spawned by a Git command that opens\nand waits for user input (e.g. \"git rebase -i\"), then the editor window\nmight be obscured by other windows. The user may be left staring at the\noriginal Git terminal window without even realizing that s/he needs to\ninteract with another window before Git can proceed. To this user Git\nappears hanging.\n\nPrint a message that Git is waiting for editor input in the original\nterminal and get rid of it when the editor returns.\n\nNo message is printed in a \"dumb\" terminal as it would not be possible\nto remove the message after the editor returns. This should not be a\nproblem as this feature is targeted at novice Git users and they are\nunlikely to work with a \"dumb\" terminal.\n\nPower users might not want to see this message or their editor might\nalready print such a message (e.g. emacsclient). Allow these users to\nsuppress the message by disabling the \"advice.waitingForEditor\" config.\n\nThe standard advise() function is not used here as it would always add\na newline which would make deleting the message harder.\n\nHelped-by: Junio C Hamano \nSigned-off-by: Lars Schneider \n---\n Documentation/config.txt | 3 +++\n advice.c | 2 ++\n advice.h | 1 +\n editor.c | 15 +++++++++++++++\n 4 files changed, 21 insertions(+)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 671fcbaa0f..de64201e82 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -354,6 +354,9 @@ advice.*::\n \tignoredHook::\n \t\tAdvice shown if an hook is ignored because the hook is not\n \t\tset as executable.\n+\twaitingForEditor::\n+\t\tPrint a message to the terminal whenever Git is waiting for\n+\t\teditor input from the user.\n --\n \n core.fileMode::\ndiff --git a/advice.c b/advice.c\nindex c6169bcb52..406efc183b 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -18,6 +18,7 @@ int advice_object_name_warning = 1;\n int advice_rm_hints = 1;\n int advice_add_embedded_repo = 1;\n int advice_ignored_hook = 1;\n+int advice_waiting_for_editor = 1;\n \n static struct {\n \tconst char *name;\n@@ -40,6 +41,7 @@ static struct {\n \t{ \"rmhints\", &advice_rm_hints },\n \t{ \"addembeddedrepo\", &advice_add_embedded_repo },\n \t{ \"ignoredhook\", &advice_ignored_hook },\n+\t{ \"waitingforeditor\", &advice_waiting_for_editor },\n \n \t/* make this an alias for backward compatibility */\n \t{ \"pushnonfastforward\", &advice_push_update_rejected }\ndiff --git a/advice.h b/advice.h\nindex f525d6f89c..70568fa792 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -20,6 +20,7 @@ extern int advice_object_name_warning;\n extern int advice_rm_hints;\n extern int advice_add_embedded_repo;\n extern int advice_ignored_hook;\n+extern int advice_waiting_for_editor;\n \n int git_default_advice_config(const char *var, const char *value);\n __attribute__((format (printf, 1, 2)))\ndiff --git a/editor.c b/editor.c\nindex c65ea698eb..cdad4f74ec 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -45,6 +45,13 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tconst char *args[] = { editor, real_path(path), NULL };\n \t\tstruct child_process p = CHILD_PROCESS_INIT;\n \t\tint ret, sig;\n+\t\tint print_waiting_for_editor = advice_waiting_for_editor &&\n+\t\t\tisatty(2) && !is_terminal_dumb();\n+\n+\t\tif (print_waiting_for_editor) {\n+\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n+\t\t\tfflush(stderr);\n+\t\t}\n \n \t\tp.argv = args;\n \t\tp.env = env;\n@@ -63,6 +70,14 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en\n \t\tif (ret)\n \t\t\treturn error(\"There was a problem with the editor '%s'.\",\n \t\t\t\t\teditor);\n+\n+\t\tif (print_waiting_for_editor)\n+\t\t\t/*\n+\t\t\t * go back to the beginning and erase the\n+\t\t\t * entire line to avoid wasting the vertical\n+\t\t\t * space.\n+\t\t\t */\n+\t\t\tfputs(\"\\r\\033[K\", stderr);\n \t}\n \n \tif (!buffer)\n-- \n2.15.0\n\n"},{"id":"333794","messageId":"20171129183514.wze5trxjfgqxqs7z@laptop.local","threadId":"47344","inReplyTo":"20171129143752.60553-1-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Thomas Adam","fromEmail":"thomas@xteddy.org","sentAt":"2017-11-29T18:35:16Z","receivedAt":"2017-11-29T18:35:29Z","isPatch":true,"sender":{"key":"thomas@xteddy.org","avatar":"https://gravatar.com/avatar/e7256db4738e501e5d2e84f00bb0bd99503165729573848a03330301fc2adc4a?d=mp&s=160"},"body":"On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n> +\t\tif (print_waiting_for_editor) {\n> +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> \t\t\tfflush(stderr);\n\nJust FYI, stderr is typically unbuffered on most systems I've used, and\nalthough the call to fflush() is harmless, I suspect it's not having any\neffect. That said, there's plenty of other places in Git which seems to think\nfflush()ing stderr actually does something.\n\n-- Thomas Adam\n"},{"id":"333850","messageId":"C91E8F24-DD13-4FEC-BAB3-A8B8F2DBBA8D@gmail.com","threadId":"47344","inReplyTo":"20171129183514.wze5trxjfgqxqs7z@laptop.local","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-11-30T13:55:35Z","receivedAt":"2017-11-30T13:55:43Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 29 Nov 2017, at 19:35, Thomas Adam wrote:\n> \n> On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n>> +\t\tif (print_waiting_for_editor) {\n>> +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n>> \t\t\tfflush(stderr);\n> \n> Just FYI, stderr is typically unbuffered on most systems I've used, and\n> although the call to fflush() is harmless, I suspect it's not having any\n> effect. That said, there's plenty of other places in Git which seems to think\n> fflush()ing stderr actually does something.\n\nI agree with the \"unbuffered\" statement. I am surprised that you expect fflush()\nto do nothing in that situation... but I am no expert in that area. Can you\npoint me to some documentation?\n\nIn any way, would all this be a problem here? The worst that could happen would\nbe that the user would not see the message, right?\n\nAre you aware of stderr usage in Git that could cause more trouble?\n\n- Lars\n"},{"id":"333852","messageId":"20171130144255.5w72jcpz4a5igiro@laptop","threadId":"47344","inReplyTo":"C91E8F24-DD13-4FEC-BAB3-A8B8F2DBBA8D@gmail.com","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Thomas Adam","fromEmail":"thomas@xteddy.org","sentAt":"2017-11-30T14:42:57Z","receivedAt":"2017-11-30T14:43:05Z","isPatch":true,"sender":{"key":"thomas@xteddy.org","avatar":"https://gravatar.com/avatar/e7256db4738e501e5d2e84f00bb0bd99503165729573848a03330301fc2adc4a?d=mp&s=160"},"body":"On Thu, Nov 30, 2017 at 02:55:35PM +0100, Lars Schneider wrote:\n> \n> > On 29 Nov 2017, at 19:35, Thomas Adam wrote:\n> > \n> > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n> >> +\t\tif (print_waiting_for_editor) {\n> >> +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> >> \t\t\tfflush(stderr);\n> > \n> > Just FYI, stderr is typically unbuffered on most systems I've used, and\n> > although the call to fflush() is harmless, I suspect it's not having any\n> > effect. That said, there's plenty of other places in Git which seems to think\n> > fflush()ing stderr actually does something.\n> \n> I agree with the \"unbuffered\" statement. I am surprised that you expect fflush()\n> to do nothing in that situation... but I am no expert in that area. Can you\n> point me to some documentation?\n\nBecause stderr is unbuffered, it will get printed immediately.\n\n> In any way, would all this be a problem here? The worst that could happen would\n> be that the user would not see the message, right?\n\nCorrect -- I only bring this up because your interdiff showed you added the\nfflush() call and I was merely pointing that out. I don't expect you to\nchange it.\n\n> Are you aware of stderr usage in Git that could cause more trouble?\n\nNo. It'll all be fine.\n\n-- Thomas Adam\n"},{"id":"333853","messageId":"87a7z3re9a.fsf@linux-m68k.org","threadId":"47344","inReplyTo":"20171130144255.5w72jcpz4a5igiro@laptop","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2017-11-30T15:13:21Z","receivedAt":"2017-11-30T15:13:33Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"On Nov 30 2017, Thomas Adam wrote:\n\n> On Thu, Nov 30, 2017 at 02:55:35PM +0100, Lars Schneider wrote:\n>> \n>> > On 29 Nov 2017, at 19:35, Thomas Adam wrote:\n>> > \n>> > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n>> >> +\t\tif (print_waiting_for_editor) {\n>> >> +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n>> >> \t\t\tfflush(stderr);\n>> > \n>> > Just FYI, stderr is typically unbuffered on most systems I've used, and\n>> > although the call to fflush() is harmless, I suspect it's not having any\n>> > effect. That said, there's plenty of other places in Git which seems to think\n>> > fflush()ing stderr actually does something.\n>> \n>> I agree with the \"unbuffered\" statement. I am surprised that you expect fflush()\n>> to do nothing in that situation... but I am no expert in that area. Can you\n>> point me to some documentation?\n>\n> Because stderr is unbuffered, it will get printed immediately.\n\nPOSIX only requires stderr to be \"not fully buffered\". If it is line\nbuffered, the message may not appear immediately.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"333866","messageId":"20171130201217.GA3578@sigill.intra.peff.net","threadId":"47344","inReplyTo":"20171129183514.wze5trxjfgqxqs7z@laptop.local","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-30T20:12:17Z","receivedAt":"2017-11-30T20:12:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 29, 2017 at 06:35:16PM +0000, Thomas Adam wrote:\n\n> On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n> > +\t\tif (print_waiting_for_editor) {\n> > +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> > \t\t\tfflush(stderr);\n> \n> Just FYI, stderr is typically unbuffered on most systems I've used, and\n> although the call to fflush() is harmless, I suspect it's not having any\n> effect. That said, there's plenty of other places in Git which seems to think\n> fflush()ing stderr actually does something.\n\nI'd prefer to keep them (including this one), even if they are noops on\nmost platforms, because:\n\n 1. They serve as a note for readers of the code that it's important\n for the output to have been printed immediately.\n\n 2. We build on some funny and antique platforms. I wouldn't be\n surprised if there's one that line buffers by default. Or even a\n modern system with funny settings (e.g., using the GNU stdbuf\n tool).\n\n(I know you said later you don't think this case needs to be removed,\nbut I want to make it clear I think it's a reasonable project-wide\npolicy to not assume we we know how stderr is buffered).\n\n-Peff\n"},{"id":"333867","messageId":"20171130203042.GB3313@sigill.intra.peff.net","threadId":"47344","inReplyTo":"20171129143752.60553-2-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 1/2] refactor \"dumb\" terminal determination","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-30T20:30:42Z","receivedAt":"2017-11-30T20:30:48Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 29, 2017 at 03:37:51PM +0100, lars.schneider@autodesk.com wrote:\n\n> From: Lars Schneider \n> \n> Move the code to detect \"dumb\" terminals into a single location. This\n> avoids duplicating the terminal detection code yet again in a subsequent\n> commit.\n\nMakes sense, and probably worth doing even if we don't follow through on\n2/2.\n\nThe patch itself looks good to me.\n\n-Peff\n"},{"id":"333868","messageId":"20171130205058.o6e2ggnjpadaxu46@laptop.local","threadId":"47344","inReplyTo":"20171130201217.GA3578@sigill.intra.peff.net","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Thomas Adam","fromEmail":"thomas@xteddy.org","sentAt":"2017-11-30T20:51:00Z","receivedAt":"2017-11-30T20:51:10Z","isPatch":true,"sender":{"key":"thomas@xteddy.org","avatar":"https://gravatar.com/avatar/e7256db4738e501e5d2e84f00bb0bd99503165729573848a03330301fc2adc4a?d=mp&s=160"},"body":"On Thu, Nov 30, 2017 at 03:12:17PM -0500, Jeff King wrote:\n> On Wed, Nov 29, 2017 at 06:35:16PM +0000, Thomas Adam wrote:\n> \n> > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n> > > +\t\tif (print_waiting_for_editor) {\n> > > +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> > > \t\t\tfflush(stderr);\n> > \n> > Just FYI, stderr is typically unbuffered on most systems I've used, and\n> > although the call to fflush() is harmless, I suspect it's not having any\n> > effect. That said, there's plenty of other places in Git which seems to think\n> > fflush()ing stderr actually does something.\n> \n> I'd prefer to keep them (including this one), even if they are noops on\n> most platforms, because:\n> \n> 1. They serve as a note for readers of the code that it's important\n> for the output to have been printed immediately.\n> \n> 2. We build on some funny and antique platforms. I wouldn't be\n> surprised if there's one that line buffers by default. Or even a\n> modern system with funny settings (e.g., using the GNU stdbuf\n> tool).\n> \n> (I know you said later you don't think this case needs to be removed,\n> but I want to make it clear I think it's a reasonable project-wide\n> policy to not assume we we know how stderr is buffered).\n\nWe're talking past each other, Peff. I'm agreeing with you. I was surprised\nto see the introduction of fflush(stderr) in the interdiff, when it wasn't\npresent before, was curious to understand why. I've done that, and since\nstated it's fine to leave it as-is.\n\n-- Thomas Adam\n"},{"id":"333869","messageId":"20171130205137.GC3313@sigill.intra.peff.net","threadId":"47344","inReplyTo":"20171129143752.60553-3-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-11-30T20:51:37Z","receivedAt":"2017-11-30T20:51:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schneider@autodesk.com wrote:\n\n> No message is printed in a \"dumb\" terminal as it would not be possible\n> to remove the message after the editor returns. This should not be a\n> problem as this feature is targeted at novice Git users and they are\n> unlikely to work with a \"dumb\" terminal.\n\nI think novice users could end up in this situation with something like:\n\n ssh remote_host git commit\n\nBut then I'd expect most terminal-based editors to give some sort of\nerror in that situation, too. And at any rate, the worst case is that\nthey get no special \"waiting...\" message from Git, which is already the\nstatus quo. So it's probably not worth worrying about such an obscure\ncase.\n\n> Power users might not want to see this message or their editor might\n> already print such a message (e.g. emacsclient). Allow these users to\n> suppress the message by disabling the \"advice.waitingForEditor\" config.\n\nI'm happy to see the hard-coded emacsclient behavior go. Hopefully we\nwon't see too many complaints about people having to set the advice\nflag.\n\n> The standard advise() function is not used here as it would always add\n> a newline which would make deleting the message harder.\n\nI tried to think of ways this \"show a message and then delete it\" could\ngo wrong. It should work OK with editors that just do curses-like\nthings, taking over the terminal and then restoring it at the end.\n\nIt does behave in a funny way if the editor produces actual lines of\noutput outside of the curses handling. E.g. (I just quit vim\nimmediately, hence the aborting message):\n\n $ GIT_EDITOR='echo foo; vim' git commit\n hint: Waiting for your editor input...foo\n Aborting commit due to empty commit message.\n\nour \"foo\" gets tacked onto the hint line, and then our deletion does\nnothing (because the newline after \"foo\" bumped us to a new line, and\nthere was nothing on that line to erase).\n\nAn even worse case (and yes, this is really reaching) is:\n\n $ GIT_EDITOR='echo one; printf \"two\\\\r\"; vim' git commit\n hint: Waiting for your editor input...one\n Aborting commit due to empty commit message.\n\nThere we ate the \"two\" line.\n\nThese are obviously the result of devils-advocate poking at the feature.\nI doubt any editor would end its output with a CR. But the first case is\nprobably going to be common, especially for actual graphical editors. We\nknow that emacsclient prints its own line, and I wouldn't be surprised\nif other graphical editors spew some telemetry to stderr (certainly\nanything built against GTK tends to do so).\n\nI don't think there's a good way around it. Portably saying \"delete\n_this_ line that I wrote earlier\" would probably require libcurses or\nsimilar. So maybe we just live with it. The deletion magic makes the\ncommon cases better (a terminal editor that doesn't print random\nlines, or a graphical editor that is quiet), and everyone else can flip\nthe advice switch if they need to. I dunno.\n\n> ---\n> Documentation/config.txt | 3 +++\n> advice.c | 2 ++\n> advice.h | 1 +\n> editor.c | 15 +++++++++++++++\n> 4 files changed, 21 insertions(+)\n\nThe patch itself looks fine, as far as correctly implementing the\ndesign.\n\n-Peff\n"},{"id":"333889","messageId":"a9786a9f-66c9-9f1e-75c8-b2c5a81b08aa@gmail.com","threadId":"47344","inReplyTo":"20171129143752.60553-2-lars.schneider@autodesk.com","subject":"Re: [PATCH v4 1/2] refactor \"dumb\" terminal determination","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-01T03:26:43Z","receivedAt":"2017-12-01T03:26:55Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Wednesday 29 November 2017 08:07 PM, lars.schneider@autodesk.com wrote:\n> +int is_terminal_dumb(void)\n> +{\n> +\tconst char *terminal = getenv(\"TERM\");\n> +\treturn !terminal || !strcmp(terminal, \"dumb\");\n\nSo, IIUC, there terminal is considered to be 'dumb' when the TERM \nenvironment variable is NOT set or when it is set to 'dumb'.\n\n> +}\n> +\n> const char *git_editor(void)\n> {\n> \tconst char *editor = getenv(\"GIT_EDITOR\");\n> -\tconst char *terminal = getenv(\"TERM\");\n> -\tint terminal_is_dumb = !terminal || !strcmp(terminal, \"dumb\");\n> +\tint terminal_is_dumb = is_terminal_dumb();\n> \n> \tif (!editor && editor_program)\n> \t\teditor = editor_program;\n> diff --git a/sideband.c b/sideband.c\n> index 1e4d684d6c..6d7f943e43 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -20,13 +20,12 @@\n> \n> int recv_sideband(const char *me, int in_stream, int out)\n> {\n> -\tconst char *term, *suffix;\n> +\tconst char *suffix;\n> \tchar buf[LARGE_PACKET_MAX + 1];\n> \tstruct strbuf outbuf = STRBUF_INIT;\n> \tint retval = 0;\n> \n> -\tterm = getenv(\"TERM\");\n> -\tif (isatty(2) && term && strcmp(term, \"dumb\"))\n> +\tif (isatty(2) && !is_terminal_dumb())\n> \t\tsuffix = ANSI_SUFFIX;\n> \telse\n> \t\tsuffix = DUMB_SUFFIX;\n> \n\nThis one looks good to me if my observation above is correct.\n\n---\nKaartic\n"},{"id":"333893","messageId":"1512099716.7882.2.camel@gmail.com","threadId":"47344","inReplyTo":"87a7z3re9a.fsf@linux-m68k.org","subject":"Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-01T03:41:56Z","receivedAt":"2017-12-01T03:42:10Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Thu, 2017-11-30 at 16:13 +0100, Andreas Schwab wrote:\n> On Nov 30 2017, Thomas Adam wrote:\n> \n> > On Thu, Nov 30, 2017 at 02:55:35PM +0100, Lars Schneider wrote:\n> > > \n> > > > On 29 Nov 2017, at 19:35, Thomas Adam wrote:\n> > > > \n> > > > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schneider@autodesk.com wrote:\n> > > > > +\t\tif (print_waiting_for_editor) {\n> > > > > +\t\t\tfprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> > > > > \t\t\tfflush(stderr);\n> > > > \n> > > > Just FYI, stderr is typically unbuffered on most systems I've used, and\n> > > > although the call to fflush() is harmless, I suspect it's not having any\n> > > > effect. That said, there's plenty of other places in Git which seems to think\n> > > > fflush()ing stderr actually does something.\n> > > \n> > > I agree with the \"unbuffered\" statement. I am surprised that you expect fflush()\n> > > to do nothing in that situation... but I am no expert in that area. Can you\n> > > point me to some documentation?\n> > \n> > Because stderr is unbuffered, it will get printed immediately.\n> \n> POSIX only requires stderr to be \"not fully buffered\". If it is line\n> buffered, the message may not appear immediately.\n> \n\nI guess Junio's reply for the same \"unbuffered\" question I asked for an\nearlier version of this patch (now, a series) might be relevant here,\n\n> > Being curious again, is flushing 'stderr' required ? AFAIK, 'stderr'\n> > is unbuffered by default and I didn't notice any calls that changed\n> > the buffering mode of it along this code path.\n> \n> \"By default\" is the key phrase. The code is merely being defensive\n> to changes in other area of the code.\n\ncf. \n\n\n-- \nKaartic\n"},{"id":"333898","messageId":"2f8d6037-b7cf-64e0-3cb6-6b684faa3144@gmail.com","threadId":"47344","inReplyTo":"20171130205137.GC3313@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-01T03:56:58Z","receivedAt":"2017-12-01T03:57:11Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Friday 01 December 2017 02:21 AM, Jeff King wrote:\n\n> These are obviously the result of devils-advocate poking at the feature.\n> I doubt any editor would end its output with a CR. But the first case is\n> probably going to be common, especially for actual graphical editors. We\n> know that emacsclient prints its own line, and I wouldn't be surprised\n> if other graphical editors spew some telemetry to stderr (certainly\n> anything built against GTK tends to do so).\n> \n\nYeah, at times 'gedit' does do what you say. And if the user \n(surprisingly!) uses an IDE such as \"eclipse\" or a hackable text editor \n\"atom\" (of course with the '-f' option) for entering his commit message \nit is likely to happen all the time for him.\n\n\n> I don't think there's a good way around it. Portably saying \"delete\n> _this_ line that I wrote earlier\" would probably require libcurses or\n> similar. So maybe we just live with it. The deletion magic makes the\n> common cases better (a terminal editor that doesn't print random\n> lines, or a graphical editor that is quiet), and everyone else can flip\n> the advice switch if they need to. I dunno.\n> \n\n---\nKaartic\n"},{"id":"333906","messageId":"BB878570-F214-4BAA-AB58-4A9C0F8F80AB@gmail.com","threadId":"47344","inReplyTo":"20171130205137.GC3313@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-01T12:52:14Z","receivedAt":"2017-12-01T12:52:25Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 30 Nov 2017, at 21:51, Jeff King wrote:\n> \n> On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schneider@autodesk.com wrote:\n> ...\n>> The standard advise() function is not used here as it would always add\n>> a newline which would make deleting the message harder.\n> \n> I tried to think of ways this \"show a message and then delete it\" could\n> go wrong. It should work OK with editors that just do curses-like\n> things, taking over the terminal and then restoring it at the end.\n> \n> It does behave in a funny way if the editor produces actual lines of\n> output outside of the curses handling. E.g. (I just quit vim\n> immediately, hence the aborting message):\n> \n> $ GIT_EDITOR='echo foo; vim' git commit\n> hint: Waiting for your editor input...foo\n> Aborting commit due to empty commit message.\n> \n> our \"foo\" gets tacked onto the hint line, and then our deletion does\n> nothing (because the newline after \"foo\" bumped us to a new line, and\n> there was nothing on that line to erase).\n> \n> An even worse case (and yes, this is really reaching) is:\n> \n> $ GIT_EDITOR='echo one; printf \"two\\\\r\"; vim' git commit\n> hint: Waiting for your editor input...one\n> Aborting commit due to empty commit message.\n> \n> There we ate the \"two\" line.\n> \n> These are obviously the result of devils-advocate poking at the feature.\n> I doubt any editor would end its output with a CR. But the first case is\n> probably going to be common, especially for actual graphical editors. We\n> know that emacsclient prints its own line, and I wouldn't be surprised\n> if other graphical editors spew some telemetry to stderr (certainly\n> anything built against GTK tends to do so).\n\nTrue! However, if a Git user is not bothered by a graphical editor that\nspews telemetry to stderr, then I think the same user wouldn't be\nbothered by one additional line printed by Git either, right?\n\n\n> The patch itself looks fine, as far as correctly implementing the\n> design.\n\nThanks for the review :-)\n\n- Lars\n\n"},{"id":"333932","messageId":"20171201182924.GB27688@sigill.intra.peff.net","threadId":"47344","inReplyTo":"BB878570-F214-4BAA-AB58-4A9C0F8F80AB@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-01T18:29:24Z","receivedAt":"2017-12-01T18:29:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:\n\n> > These are obviously the result of devils-advocate poking at the feature.\n> > I doubt any editor would end its output with a CR. But the first case is\n> > probably going to be common, especially for actual graphical editors. We\n> > know that emacsclient prints its own line, and I wouldn't be surprised\n> > if other graphical editors spew some telemetry to stderr (certainly\n> > anything built against GTK tends to do so).\n> \n> True! However, if a Git user is not bothered by a graphical editor that\n> spews telemetry to stderr, then I think the same user wouldn't be\n> bothered by one additional line printed by Git either, right?\n\nYeah, if there's a lot of spew, I agree it probably doesn't matter.\n\nThe \"emacsclient\" one is probably the worst case, because it would print\na single line of its own, which would get tacked onto the \"Waiting...\"\nmessage printed by Git, since it doesn't end with a newline.\n\n> > The patch itself looks fine, as far as correctly implementing the\n> > design.\n> \n> Thanks for the review :-)\n\nActually, I meant to bikeshed one part but forgot. ;)\n\n> + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n\nI found \"waiting for editor input\" to be a funny way of saying this. I\ninput to the editor, the editor does not input to Git. :)\n\nMaybe \"waiting for your editor finish\" or something would make more\nsense?\n\nOr given that the goal is really just making it clear that we've spawned\nan editor, something like \"starting editor %s...\" would work. I think\nthe \"waiting for...\" pattern is perfectly fine, though.\n\n-Peff\n"},{"id":"333949","messageId":"aacb7604-edc3-a920-da43-701b57045956@gmail.com","threadId":"47344","inReplyTo":"20171201182924.GB27688@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-02T03:45:49Z","receivedAt":"2017-12-02T03:46:17Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Friday 01 December 2017 11:59 PM, Jeff King wrote:\n> On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:\n>>\n>> Thanks for the review :-)\n> \n> Actually, I meant to bikeshed one part but forgot. ;)\n> \n>> + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> \n> I found \"waiting for editor input\" to be a funny way of saying this. I\n> input to the editor, the editor does not input to Git. :)\n> \n> Maybe \"waiting for your editor finish\" or something would make more\n> sense?\n\nMay be the good \"Launched editor. Waiting ...\" message, that was used in \na previous version, itself makes sense?\n\n\n> \n> Or given that the goal is really just making it clear that we've spawned\n> an editor, something like \"starting editor %s...\" would work.\n\nThere was already discussion related to the \"continuous tense\" used in \nthe phrase.\n\nExtract from [1]:\n\n-- 8< --\n > fprintf(stderr, \"Launching your editor...\");\n\n\"It takes quite some time to launch this special Git Editor\"\n\nAs Lars pointed out, the editor may be launched in the background,\nthat the user would not know, but they might expect a thing to\npop up as a modal dialog as is always with UIs.\n\nSo despite it being technically wrong at this point in time,\nI would phrase it in past tense or in a way that indicates that the\nuser needs to take action already.\n\nThe \"Launching...\" sounds as if I need to wait for an event to occur.\n-- >8 --\n\n[1]: \nhttps://public-inbox.org/git/CAGZ79kZbm8SGY4rXKZHV82E-HX9qbQ4iyCbMgJEBFQf4fj3u=Q@mail.gmail.com/\n\n\n> I think\n> the \"waiting for...\" pattern is perfectly fine, though.\n>\n"},{"id":"333984","messageId":"xmqqvahojssu.fsf@gitster.mtv.corp.google.com","threadId":"47344","inReplyTo":"20171130205137.GC3313@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-03T05:15:29Z","receivedAt":"2017-12-03T05:15:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I tried to think of ways this \"show a message and then delete it\" could\n> go wrong. It should work OK with editors that just do curses-like\n> things, taking over the terminal and then restoring it at the end.\n> ...\n\nI think that it is not worth to special-case \"dumb\" terminals like\nthis round of the patches do. If it so much disturbs reviewers that\n\"\\e[K\" may not work everywhere, we can do without the \"then delete\nit\" part. It was merely trying to be extra nice, and the more\nimportant part of the \"feature\" is to be noticeable, and I do think\nthat not showing anything on \"dumb\", only because the message cannot\nbe retracted, is putting the cart before the horse. \n\nSince especially now people are hiding this behind an advise.*\nthing, I think it is OK to show a message and waste a line, even.\n\n> An even worse case (and yes, this is really reaching) is:\n>\n> $ GIT_EDITOR='echo one; printf \"two\\\\r\"; vim' git commit\n> hint: Waiting for your editor input...one\n> Aborting commit due to empty commit message.\n>\n> There we ate the \"two\" line.\n\nYes, I would have to agree that this one is reaching, as there isn't\nany valid reason other than \"the editor then wanted to do \\e[K\nlater\" for it to end its last line with CR. So our eating that line\nis not a problem.\n"},{"id":"333994","messageId":"D3A5C7E6-A63C-431C-97B5-070632EE3540@gmail.com","threadId":"47344","inReplyTo":"xmqqvahojssu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-03T12:47:04Z","receivedAt":"2017-12-03T12:51:28Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 03 Dec 2017, at 06:15, Junio C Hamano wrote:\n> \n> Jeff King writes:\n> \n>> I tried to think of ways this \"show a message and then delete it\" could\n>> go wrong. It should work OK with editors that just do curses-like\n>> things, taking over the terminal and then restoring it at the end.\n>> ...\n> \n> I think that it is not worth to special-case \"dumb\" terminals like\n> this round of the patches do. If it so much disturbs reviewers that\n> \"\\e[K\" may not work everywhere, we can do without the \"then delete\n> it\" part. It was merely trying to be extra nice, and the more\n> important part of the \"feature\" is to be noticeable, and I do think\n> that not showing anything on \"dumb\", only because the message cannot\n> be retracted, is putting the cart before the horse. \n> \n> Since especially now people are hiding this behind an advise.*\n> thing, I think it is OK to show a message and waste a line, even.\n\nWell, my reasoning was to minimize the risk of bothering people:\nPeople using \"dumb\" terminals wouldn't be bothered because nothing\nchanges and people using \"smart\" terminals would see the message\nonly temporarily. Of course, emacsclient users would be the\nexception. They would always be bothered and therefore I added\nthe \"advice\" switch.\n\nThat being said, I can also follow your logic. If we have such\na feature then we shouldn't surprise the user. We should make it \nconsistently available in all environments.\n\nI am on the fence here. I like consistency but I don't want to\nbother Git users.\n\n@Peff: Do you lean into either direction? Could you imagine that\nnovice/regular users are bothered? (I don't expect experts to be\nbothered too much as they would likely be able to find and set \nthe advice config).\n\n\nThanks,\nLars\n"},{"id":"333998","messageId":"D330CEDD-7F6B-49DB-A30E-EE0BDB80E28A@gmail.com","threadId":"47344","inReplyTo":"aacb7604-edc3-a920-da43-701b57045956@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-03T16:39:10Z","receivedAt":"2017-12-03T16:39:17Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 02 Dec 2017, at 04:45, Kaartic Sivaraam wrote:\n> \n> On Friday 01 December 2017 11:59 PM, Jeff King wrote:\n>> On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:\n>>> \n>>> Thanks for the review :-)\n>> Actually, I meant to bikeshed one part but forgot. ;)\n>>> + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n>> I found \"waiting for editor input\" to be a funny way of saying this. I\n>> input to the editor, the editor does not input to Git. :)\n>> Maybe \"waiting for your editor finish\" or something would make more\n>> sense?\n> \n> May be the good \"Launched editor. Waiting ...\" message, that was used in a previous version, itself makes sense?\n\nPerfect bikeshed topic :-)\n\nI would like to add \"for your input\" or \"for you\" to convey \nthat Git is not waiting for the machine but for the user.\n\n \"hint: Launched editor. Waiting for your input...\"\n\nWould that work for you?\n\nThanks,\nLars"},{"id":"334062","messageId":"1512405704.15792.2.camel@gmail.com","threadId":"47344","inReplyTo":"D330CEDD-7F6B-49DB-A30E-EE0BDB80E28A@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2017-12-04T16:41:44Z","receivedAt":"2017-12-04T16:42:22Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On Sun, 2017-12-03 at 17:39 +0100, Lars Schneider wrote:\n> > On 02 Dec 2017, at 04:45, Kaartic Sivaraam wrote:\n> > \n> > On Friday 01 December 2017 11:59 PM, Jeff King wrote:\n> > > On Fri, Dec 01, 2017 at 01:52:14PM +0100, Lars Schneider wrote:\n> > > > \n> > > > Thanks for the review :-)\n> > > \n> > > Actually, I meant to bikeshed one part but forgot. ;)\n> > > > + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> > > \n> > > I found \"waiting for editor input\" to be a funny way of saying this. I\n> > > input to the editor, the editor does not input to Git. :)\n> > > Maybe \"waiting for your editor finish\" or something would make more\n> > > sense?\n> > \n> > May be the good \"Launched editor. Waiting ...\" message, that was used in a previous version, itself makes sense?\n> \n> Perfect bikeshed topic :-)\n> \n\nYep :-)\n\n\n> I would like to add \"for your input\" or \"for you\" to convey \n> that Git is not waiting for the machine but for the user.\n> \n> \"hint: Launched editor. Waiting for your input...\"\n> \n> Would that work for you?\n> \n\nYeah, this one does look fine.\n\nThat said, FWIW I don't have strong opinions about the phrase/sentence\nexcept that they should be readable and shouldn't get too verbose.\n\n\nThanks, \nKaartic\n"},{"id":"334069","messageId":"20171204172504.GC13332@sigill.intra.peff.net","threadId":"47344","inReplyTo":"aacb7604-edc3-a920-da43-701b57045956@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T17:25:04Z","receivedAt":"2017-12-04T17:25:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 02, 2017 at 09:15:49AM +0530, Kaartic Sivaraam wrote:\n\n> > Or given that the goal is really just making it clear that we've spawned\n> > an editor, something like \"starting editor %s...\" would work.\n> \n> There was already discussion related to the \"continuous tense\" used in the\n> phrase.\n> \n> Extract from [1]:\n\nThanks, I missed that one. I agree with the argument there.\n\n-Peff\n"},{"id":"334071","messageId":"20171204172623.GD13332@sigill.intra.peff.net","threadId":"47344","inReplyTo":"D330CEDD-7F6B-49DB-A30E-EE0BDB80E28A@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T17:26:23Z","receivedAt":"2017-12-04T17:26:29Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 03, 2017 at 05:39:10PM +0100, Lars Schneider wrote:\n\n> >>> + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n> >> I found \"waiting for editor input\" to be a funny way of saying this. I\n> >> input to the editor, the editor does not input to Git. :)\n> >> Maybe \"waiting for your editor finish\" or something would make more\n> >> sense?\n> > \n> > May be the good \"Launched editor. Waiting ...\" message, that was used in a previous version, itself makes sense?\n> \n> Perfect bikeshed topic :-)\n> \n> I would like to add \"for your input\" or \"for you\" to convey \n> that Git is not waiting for the machine but for the user.\n> \n> \"hint: Launched editor. Waiting for your input...\"\n> \n> Would that work for you?\n\nI guess \"input\" was the part that I found funny/confusing. The only\nthing we know is that we're waiting on the editor process to finish, and\neverything else is making assumptions about what's happening in the\neditor.\n\n-Peff\n"},{"id":"334072","messageId":"20171204173024.GE13332@sigill.intra.peff.net","threadId":"47344","inReplyTo":"xmqqvahojssu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T17:30:24Z","receivedAt":"2017-12-04T17:30:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 02, 2017 at 09:15:29PM -0800, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I tried to think of ways this \"show a message and then delete it\" could\n> > go wrong. It should work OK with editors that just do curses-like\n> > things, taking over the terminal and then restoring it at the end.\n> > ...\n> \n> I think that it is not worth to special-case \"dumb\" terminals like\n> this round of the patches do. If it so much disturbs reviewers that\n> \"\\e[K\" may not work everywhere, we can do without the \"then delete\n> it\" part. It was merely trying to be extra nice, and the more\n> important part of the \"feature\" is to be noticeable, and I do think\n> that not showing anything on \"dumb\", only because the message cannot\n> be retracted, is putting the cart before the horse. \n> \n> Since especially now people are hiding this behind an advise.*\n> thing, I think it is OK to show a message and waste a line, even.\n\nYeah, I was tempted to suggest just dropping this terminal magic\ncompletely. But it probably _does_ work and is helpful in the majority\nof cases (i.e., where people have in-terminal editors). I dunno.\n\nI am a little wary of hiding behind \"but you can disable it with a\nconfig option\", because that's still a thing that users have to actually\ndo to get the previous behavior. And I expect to get some \"ugh, git is\ntoo chatty and annoying\" backlash once this is in a released version.\n\nBut maybe that is just being paranoid. It's not like we don't have a lot\nof other advice flags. I really could go either way on this whole thing\n(but I'll be setting the advice flag myself ;) ).\n\n> > An even worse case (and yes, this is really reaching) is:\n> >\n> > $ GIT_EDITOR='echo one; printf \"two\\\\r\"; vim' git commit\n> > hint: Waiting for your editor input...one\n> > Aborting commit due to empty commit message.\n> >\n> > There we ate the \"two\" line.\n> \n> Yes, I would have to agree that this one is reaching, as there isn't\n> any valid reason other than \"the editor then wanted to do \\e[K\n> later\" for it to end its last line with CR. So our eating that line\n> is not a problem.\n\nYeah, this was just me trying to come up with all possible implications.\nI agree it's probably not worth worrying about.\n\n-Peff\n"},{"id":"334073","messageId":"20171204173223.GF13332@sigill.intra.peff.net","threadId":"47344","inReplyTo":"D3A5C7E6-A63C-431C-97B5-070632EE3540@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T17:32:23Z","receivedAt":"2017-12-04T17:32:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:\n\n> I am on the fence here. I like consistency but I don't want to\n> bother Git users.\n> \n> @Peff: Do you lean into either direction? Could you imagine that\n> novice/regular users are bothered? (I don't expect experts to be\n> bothered too much as they would likely be able to find and set \n> the advice config).\n\nI also am on the fence, and am OK with any of the options discussed.\n\nBut now I've said my reservations on the list, so I can say \"I told you\nso\" if people complain (and naturally refuse to admit my objections if\npeople love it). :)\n\n-Peff\n"},{"id":"334088","messageId":"07CF6EAE-815E-4846-887D-560F2459E69D@gmail.com","threadId":"47344","inReplyTo":"20171204172623.GD13332@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-04T21:31:15Z","receivedAt":"2017-12-04T21:31:23Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 04 Dec 2017, at 18:26, Jeff King wrote:\n> \n> On Sun, Dec 03, 2017 at 05:39:10PM +0100, Lars Schneider wrote:\n> \n>>>>> + fprintf(stderr, _(\"hint: Waiting for your editor input...\"));\n>>>> I found \"waiting for editor input\" to be a funny way of saying this. I\n>>>> input to the editor, the editor does not input to Git. :)\n>>>> Maybe \"waiting for your editor finish\" or something would make more\n>>>> sense?\n>>> \n>>> May be the good \"Launched editor. Waiting ...\" message, that was used in a previous version, itself makes sense?\n>> \n>> Perfect bikeshed topic :-)\n>> \n>> I would like to add \"for your input\" or \"for you\" to convey \n>> that Git is not waiting for the machine but for the user.\n>> \n>> \"hint: Launched editor. Waiting for your input...\"\n>> \n>> Would that work for you?\n> \n> I guess \"input\" was the part that I found funny/confusing. The only\n> thing we know is that we're waiting on the editor process to finish, and\n> everything else is making assumptions about what's happening in the\n> editor.\n\nI see. How about:\n\n\"hint: Launched editor. Waiting for your action...\"\n(my preference)\n\nor\n\n\"hint: Launched editor. Waiting for you...\"\n\n- Lars"},{"id":"334090","messageId":"599BA6A0-0CAB-4C77-8428-F7E46064DF14@gmail.com","threadId":"47344","inReplyTo":"20171204173223.GF13332@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-04T21:34:04Z","receivedAt":"2017-12-04T21:34:11Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 04 Dec 2017, at 18:32, Jeff King wrote:\n> \n> On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:\n> \n>> I am on the fence here. I like consistency but I don't want to\n>> bother Git users.\n>> \n>> @Peff: Do you lean into either direction? Could you imagine that\n>> novice/regular users are bothered? (I don't expect experts to be\n>> bothered too much as they would likely be able to find and set \n>> the advice config).\n> \n> I also am on the fence, and am OK with any of the options discussed.\n> \n> But now I've said my reservations on the list, so I can say \"I told you\n> so\" if people complain (and naturally refuse to admit my objections if\n> people love it). :)\n\nWell, since you and I are on the fence, I will follow Junio. Then we\ncan at least argue that the feature is consistent on all terminals.\n\n- Lars\n"},{"id":"334094","messageId":"xmqqr2sakw90.fsf@gitster.mtv.corp.google.com","threadId":"47344","inReplyTo":"20171204173223.GF13332@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-04T21:40:11Z","receivedAt":"2017-12-04T21:40:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Sun, Dec 03, 2017 at 01:47:04PM +0100, Lars Schneider wrote:\n>\n>> I am on the fence here. I like consistency but I don't want to\n>> bother Git users.\n>> \n>> @Peff: Do you lean into either direction? Could you imagine that\n>> novice/regular users are bothered? (I don't expect experts to be\n>> bothered too much as they would likely be able to find and set \n>> the advice config).\n>\n> I also am on the fence, and am OK with any of the options discussed.\n>\n> But now I've said my reservations on the list, so I can say \"I told you\n> so\" if people complain (and naturally refuse to admit my objections if\n> people love it). :)\n\nHeh. I am even OK with not doing anything for that matter ;-)\n\n"},{"id":"334095","messageId":"20171204214221.GB21552@sigill.intra.peff.net","threadId":"47344","inReplyTo":"07CF6EAE-815E-4846-887D-560F2459E69D@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T21:42:21Z","receivedAt":"2017-12-04T21:42:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 04, 2017 at 10:31:15PM +0100, Lars Schneider wrote:\n\n> >> I would like to add \"for your input\" or \"for you\" to convey \n> >> that Git is not waiting for the machine but for the user.\n> >> \n> >> \"hint: Launched editor. Waiting for your input...\"\n> >> \n> >> Would that work for you?\n> > \n> > I guess \"input\" was the part that I found funny/confusing. The only\n> > thing we know is that we're waiting on the editor process to finish, and\n> > everything else is making assumptions about what's happening in the\n> > editor.\n> \n> I see. How about:\n> \n> \"hint: Launched editor. Waiting for your action...\"\n> (my preference)\n> \n> or\n> \n> \"hint: Launched editor. Waiting for you...\"\n\nBetter, IMHO, though I still think literally saying:\n\n hint: Waiting for your editor to exit...\n\nis the most accurate, which I think makes it clear that you must _exit_\nyour editor, not just save and close the file.\n\nI dunno, maybe that is being overly paranoid. Certainly I have seen\ngraphical programs that have a mismatch with the one-process-per-action\nway that most terminal editors view the world, and would hang around\neven after the user thinks they are done editing. But at the same time,\nthose programs are unlikely to work well as $GIT_EDITOR in the first\nplace, because running them from the terminal may just open a new window\nin an existing session and exit immediately (which is the opposite\nproblem -- the editor exited before the user actually did their thing).\n\nSo I'm not sure if that would be a problem in practice or not. I'm too\nmired in the vim world to have any real data. Somebody like you who is\nsupporting a large number of less-Unixy users probably has more\nperspective there.\n\n-Peff\n"},{"id":"334097","messageId":"787AD800-7169-4B83-A1AD-171A15D103B9@gmail.com","threadId":"47344","inReplyTo":"20171204214221.GB21552@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2017-12-04T21:54:40Z","receivedAt":"2017-12-04T21:54:49Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\n> On 04 Dec 2017, at 22:42, Jeff King wrote:\n> \n> On Mon, Dec 04, 2017 at 10:31:15PM +0100, Lars Schneider wrote:\n> \n>>>> I would like to add \"for your input\" or \"for you\" to convey \n>>>> that Git is not waiting for the machine but for the user.\n>>>> \n>>>> \"hint: Launched editor. Waiting for your input...\"\n>>>> \n>>>> Would that work for you?\n>>> \n>>> I guess \"input\" was the part that I found funny/confusing. The only\n>>> thing we know is that we're waiting on the editor process to finish, and\n>>> everything else is making assumptions about what's happening in the\n>>> editor.\n>> \n>> I see. How about:\n>> \n>> \"hint: Launched editor. Waiting for your action...\"\n>> (my preference)\n>> \n>> or\n>> \n>> \"hint: Launched editor. Waiting for you...\"\n> \n> Better, IMHO, though I still think literally saying:\n> \n> hint: Waiting for your editor to exit...\n> \n> is the most accurate, which I think makes it clear that you must _exit_\n> your editor, not just save and close the file.\n\nI think \"exit\" would be confusing because most graphical editors (Sublime,\nTextmate, Notepad++, ...) can open multiple files and do not need to exit. \nThe requirement is indeed save and close the file.\n\nHow about:\n\n hint: Waiting for your editor to close the file...\n\nI generally like that as this is technical correct from all angles.\nMy only nit would be that \"the file\" is a bit imprecise... but\nthat's probably no problem.\n\n- Lars\n"},{"id":"334102","messageId":"20171204220908.GA8184@sigill.intra.peff.net","threadId":"47344","inReplyTo":"787AD800-7169-4B83-A1AD-171A15D103B9@gmail.com","subject":"Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2017-12-04T22:09:09Z","receivedAt":"2017-12-04T22:09:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 04, 2017 at 10:54:40PM +0100, Lars Schneider wrote:\n\n> > Better, IMHO, though I still think literally saying:\n> > \n> > hint: Waiting for your editor to exit...\n> > \n> > is the most accurate, which I think makes it clear that you must _exit_\n> > your editor, not just save and close the file.\n> \n> I think \"exit\" would be confusing because most graphical editors (Sublime,\n> Textmate, Notepad++, ...) can open multiple files and do not need to exit. \n> The requirement is indeed save and close the file.\n> \n> How about:\n> \n> hint: Waiting for your editor to close the file...\n> \n> I generally like that as this is technical correct from all angles.\n> My only nit would be that \"the file\" is a bit imprecise... but\n> that's probably no problem.\n\nOK, that makes sense. There are two definitions of \"exit\" here. We care\nabout the process exiting, but of course the editor may present a\nconcept of \"exiting\" to the user that is different.\n\nI know emacsclient behaves that way (the client process exits when the\nbuffer is closed, even though the rest of emacs may still be running),\nbut I didn't realize other editors did so. I know atom doesn't, but then\nit also exits immediately, making it inappropriate for $GIT_EDITOR in\nthe first place.\n\n-Peff\n"}]}