patch 9.2.0224: channel: 2 issues with out/err callbacks

Problem:  channel: 2 issues with out/err callbacks
Solution: fix indentation and output order with term_start()
          (Hirohito Higashi).

When term_start() is called with err_cb (and optionally out_cb),
two issues occur:
1. Unexpected indentation in the terminal buffer display: stderr
   arrives via a pipe which lacks the PTY ONLCR line discipline,
   so bare LF moves the cursor down without a CR, causing each
   subsequent line to be indented one column further.

2. stdout appears in the terminal and callbacks before stderr,
   even when the child process writes to stderr first.  This is
   because channel_parse_messages() iterates parts in enum order
   (PART_OUT before PART_ERR), so when both have buffered data
   they are always processed in the wrong order.

Solution:
- In may_invoke_callback(), before writing PART_ERR data to the
  terminal buffer, convert bare LF to CR+LF so that vterm renders
  each line at column 0.

- In channel_parse_messages(), when about to process PART_OUT of a
  terminal PTY job, if PART_ERR already has readahead data, skip
  PART_OUT and process PART_ERR first.  This works without any
  artificial delay because channel_select_check() reads all ready
  file descriptors into their readahead buffers in a single
  select() pass before any callbacks are invoked; by the time
  channel_parse_messages() runs, both buffers are populated and
  the skip logic can enforce the correct order.

- Also add a per-line split for out_cb/err_cb on terminal PTY
  jobs: instead of passing a potentially multi-line raw chunk to
  the callback, split on NL and strip trailing CR so each callback
  receives exactly one clean line.

Add Test_term_start_cb_per_line() to verify that err_cb and out_cb
each receive one line per call, with correct stderr-before-stdout
ordering, without any sleep between the writes.

fixes:  #16354
closes: #19776

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Hirohito Higashi <h.east.727@gmail.com>
Signed-off-by: Christian Brabandt <cb@256bit.org>

diff --git a/src/channel.c b/src/channel.c
index caa382da75f6637a48b1c6fa9371f87f68ca2f3c..4607b1e605e4915e5367185aaac04070e7bc3a9f 100644 (file)
--- a/src/channel.c
+++ b/src/channel.c
@@ -3529,7 +3529,43 @@ may_invoke_callback(channel_T *channel, ch_part_T part)
            {
 #ifdef FEAT_TERMINAL
                if (buffer->b_term != NULL)
-                   write_to_term(buffer, msg, channel);
+               {
+                   // PART_ERR data arrives from a pipe without PTY ONLCR
+                   // conversion, so lone NL must be converted to CR+NL to
+                   // prevent unexpected indentation in the terminal display.
+                   if (part == PART_ERR)
+                   {
+                       char_u *cp;
+                       int    extra = 0;
+
+                       for (cp = msg; *cp != NUL; ++cp)
+                           if (*cp == NL && (cp == msg || cp[-1] != CAR))
+                               ++extra;
+                       if (extra > 0)
+                       {
+                           char_u *crlf_msg = alloc(STRLEN(msg) + extra + 1);
+
+                           if (crlf_msg != NULL)
+                           {
+                               char_u *q = crlf_msg;
+
+                               for (cp = msg; *cp != NUL; ++cp)
+                               {
+                                   if (*cp == NL && (cp == msg || cp[-1] != CAR))
+                                       *q++ = CAR;
+                                   *q++ = *cp;
+                               }
+                               *q = NUL;
+                               write_to_term(buffer, crlf_msg, channel);
+                               vim_free(crlf_msg);
+                           }
+                       }
+                       else
+                           write_to_term(buffer, msg, channel);
+                   }
+                   else
+                       write_to_term(buffer, msg, channel);
+               }
                else
 #endif
                    append_to_buffer(buffer, msg, channel, part);
@@ -3545,7 +3581,46 @@ may_invoke_callback(channel_T *channel, ch_part_T part)
                // invoke the channel callback
                ch_log(channel, "Invoking channel callback %s",
                                                    (char *)callback->cb_name);
-               invoke_callback(channel, callback, argv);
+#ifdef FEAT_TERMINAL
+               // For a terminal job in RAW mode (term_start()), split msg on
+               // NL and invoke the callback once per line with trailing CR
+               // stripped.  This ensures out_cb/err_cb receive one line at a
+               // time regardless of how much data arrives in a single read.
+               if (ch_mode == CH_MODE_RAW && msg != NULL
+                       && channel->ch_job != NULL
+                       && channel->ch_job->jv_tty_out != NULL)
+               {
+                   char_u *cp = msg;
+                   char_u *nl;
+
+                   while ((nl = vim_strchr(cp, NL)) != NULL)
+                   {
+                       long_u len = (long_u)(nl - cp);
+
+                       if (len > 0 && cp[len - 1] == CAR)
+                           --len;
+                       argv[1].vval.v_string = vim_strnsave(cp, len);
+                       if (argv[1].vval.v_string != NULL)
+                           invoke_callback(channel, callback, argv);
+                       vim_free(argv[1].vval.v_string);
+                       cp = nl + 1;
+                   }
+                   if (*cp != NUL)
+                   {
+                       long_u len = STRLEN(cp);
+
+                       if (len > 0 && cp[len - 1] == CAR)
+                           --len;
+                       argv[1].vval.v_string = vim_strnsave(cp, len);
+                       if (argv[1].vval.v_string != NULL)
+                           invoke_callback(channel, callback, argv);
+                       vim_free(argv[1].vval.v_string);
+                   }
+                   argv[1].vval.v_string = msg;
+               }
+               else
+#endif
+                   invoke_callback(channel, callback, argv);
            }
        }
     }
@@ -5448,6 +5523,21 @@ channel_parse_messages(void)
            }
        }
 
+#ifdef FEAT_TERMINAL
+       // For a terminal job with a separate stderr pipe, defer processing
+       // PART_OUT until PART_ERR is drained.  Both are filled by
+       // channel_select_check() in the same select() pass, so when both
+       // have readahead the child's write order (stderr before stdout) is
+       // preserved by handling PART_ERR first.
+       if (part == PART_OUT
+               && channel->ch_job != NULL
+               && channel->ch_job->jv_tty_out != NULL
+               && channel_has_readahead(channel, PART_ERR))
+       {
+           part = PART_ERR;
+           continue;
+       }
+#endif
        if (channel->ch_part[part].ch_fd != INVALID_FD
                                      || channel_has_readahead(channel, part))
        {

diff --git a/src/testdir/test_channel.vim b/src/testdir/test_channel.vim
index 94dec0d973199385ee8c0c2ca47223d5280856a8..f35b6d3cb12bea64dd63bc2ffb7e9ac3b7d7273a 100644 (file)
--- a/src/testdir/test_channel.vim
+++ b/src/testdir/test_channel.vim
@@ -2919,4 +2919,25 @@ func Test_error_callback_terminal()
   unlet! g:out g:error
 endfunc
 
+" Verify that term_start() with out_cb/err_cb delivers one line per callback
+" call (no embedded newlines, no trailing CR), matching the user's expectation.
+func Test_term_start_cb_per_line()
+  CheckUnix
+  CheckFeature terminal
+  let g:Ch_msgs = []
+  let script_file = 'Xterm_cb_per_line.sh'
+  call writefile(["#!/bin/sh",
+        \         "printf 'err:1\\nerr:2\\n' >&2",
+        \         "printf 'out:3\\n'"], script_file, 'D')
+  call setfperm(script_file, 'rwxr-xr-x')
+  let ptybuf = term_start('./' .. script_file, {
+        \ 'out_cb': {ch, msg -> add(g:Ch_msgs, msg)},
+        \ 'err_cb': {ch, msg -> add(g:Ch_msgs, msg)}})
+  call WaitForAssert({-> assert_equal(3, len(g:Ch_msgs))}, 5000)
+  " Each line must arrive as a separate callback call with no embedded CR/NL.
+  call assert_equal(['err:1', 'err:2', 'out:3'], g:Ch_msgs)
+  call job_stop(term_getjob(ptybuf))
+  unlet g:Ch_msgs
+endfunc
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 50918467f6367934cc21ba3154e944dce45e4f98..f16dd8d62e48f7b7c271534291502630cb5b2b06 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -734,6 +734,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    224,
 /**/
     223,
 /**/