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

1 view
Skip to first unread message

Christian Brabandt

unread,
Mar 22, 2026, 12:47:08 PM (24 hours ago) Mar 22
to vim...@googlegroups.com
patch 9.2.0224: channel: 2 issues with out/err callbacks

Commit: https://github.com/vim/vim/commit/9e5547484956e3e8016ab8447170d63ad58d7568
Author: Hirohito Higashi <h.eas...@gmail.com>
Date: Sun Mar 22 16:32:07 2026 +0000

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 <nor...@anthropic.com>
Signed-off-by: Hirohito Higashi <h.eas...@gmail.com>
Signed-off-by: Christian Brabandt <c...@256bit.org>

diff --git a/src/channel.c b/src/channel.c
index caa382da7..4607b1e60 100644
--- 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 94dec0d97..f35b6d3cb 100644
--- 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 50918467f..f16dd8d62 100644
--- 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,
/**/
Reply all
Reply to author
Forward
0 new messages