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

Re: [PATCH v4 2/2] launch_editor(): indicate that Git waits for user input

From
Lars Schneider <larsxschneider@gmail.com>
Date
Dec 1, 2017, 12:52 UTC
Message-ID
<BB878570-F214-4BAA-AB58-4A9C0F8F80AB@gmail.com>
In-Reply-To
<20171130205137.GC3313@sigill.intra.peff.net>
Show 37 quoted lines
> On 30 Nov 2017, at 21:51, Jeff King <peff@peff.net> wrote:
> 
> On Wed, Nov 29, 2017 at 03:37:52PM +0100, lars.schneider@autodesk.com wrote:
> ...
>> The standard advise() function is not used here as it would always add
>> a newline which would make deleting the message harder.
> 
> I tried to think of ways this "show a message and then delete it" could
> go wrong. It should work OK with editors that just do curses-like
> things, taking over the terminal and then restoring it at the end.
> 
> It does behave in a funny way if the editor produces actual lines of
> output outside of the curses handling. E.g. (I just quit vim
> immediately, hence the aborting message):
> 
>  $ GIT_EDITOR='echo foo; vim' git commit
>  hint: Waiting for your editor input...foo
>  Aborting commit due to empty commit message.
> 
> our "foo" gets tacked onto the hint line, and then our deletion does
> nothing (because the newline after "foo" bumped us to a new line, and
> there was nothing on that line to erase).
> 
> An even worse case (and yes, this is really reaching) is:
> 
>  $ GIT_EDITOR='echo one; printf "two\\r"; vim' git commit
>  hint: Waiting for your editor input...one
>  Aborting commit due to empty commit message.
> 
> There we ate the "two" line.
> 
> These are obviously the result of devils-advocate poking at the feature.
> I doubt any editor would end its output with a CR. But the first case is
> probably going to be common, especially for actual graphical editors. We
> know that emacsclient prints its own line, and I wouldn't be surprised
> if other graphical editors spew some telemetry to stderr (certainly
> anything built against GTK tends to do so).

True! However, if a Git user is not bothered by a graphical editor that spews telemetry to stderr, then I think the same user wouldn't be bothered by one additional line printed by Git either, right?

> The patch itself looks fine, as far as correctly implementing the
> design.
Thanks for the review :-)
- Lars
Previous: Kaartic SivaraamNext: Jeff King
Message 8 of 31 in “launch_editor(): indicate that Git waits for user input”
  1. 0/2 launch_editor(): indicate that Git waits for user inputlars.schneider@autodesk.com, Nov 29, 2017
  2. 1/2 refactor "dumb" terminal determinationlars.schneider@autodesk.com, Nov 29, 2017
  3. Jeff KingNov 30, 2017
  4. Kaartic SivaraamDec 1, 2017
  5. 2/2 launch_editor(): indicate that Git waits for user inputlars.schneider@autodesk.com, Nov 29, 2017
  6. Jeff KingNov 30, 2017
  7. Kaartic SivaraamDec 1, 2017
  8. Lars SchneiderDec 1, 2017
  9. Jeff KingDec 1, 2017
  10. Kaartic SivaraamDec 2, 2017
  11. Lars SchneiderDec 3, 2017
  12. Kaartic SivaraamDec 4, 2017
  13. Jeff KingDec 4, 2017
  14. Lars SchneiderDec 4, 2017
  15. Jeff KingDec 4, 2017
  16. Lars SchneiderDec 4, 2017
  17. Jeff KingDec 4, 2017
  18. Jeff KingDec 4, 2017
  19. Junio C HamanoDec 3, 2017
  20. Lars SchneiderDec 3, 2017
  21. Jeff KingDec 4, 2017
  22. Lars SchneiderDec 4, 2017
  23. Junio C HamanoDec 4, 2017
  24. Jeff KingDec 4, 2017
  25. Thomas AdamNov 29, 2017
  26. Lars SchneiderNov 30, 2017
  27. Thomas AdamNov 30, 2017
  28. Andreas SchwabNov 30, 2017
  29. Kaartic SivaraamDec 1, 2017
  30. Jeff KingNov 30, 2017
  31. Thomas AdamNov 30, 2017

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.