| Age | Commit message (Collapse) | Author |
|
Whereas nonscrolling fifos generally[^1] append to the very end of
the buffer, scrolling fifos generally insert *before* the final
empty line.
This means that every single BufReadFifo hook in a scrolling fifo will
report an additional newline. This is clearly wrong. Even reporting
it only once would be wrong, because the newline is not added by a
fifo read.
This behavior has always existed for "edit -scroll -fifo" buffers.
For stdin buffers, it was re-introduced in c3b01a3c9 (Add back option
to scroll in stdin buffers, 2024-11-27).
Fix this by ending the reported range before the final empty line.
Handle one edge case: if the inserted range did not end with a
newline, the final empty line collapses into the previous line.
In this case we already don't report the newline because it's declared
"artificially-added" by 658915086 (Fix BufReadFifo overlapping range
on partial line, 2024-11-23).
This fixes the problem described at
https://github.com/mawww/kakoune/issues/5255#issuecomment-2505650511
Tests are copied verbatim from the no-scroll cases (not yet sure how
to share logic / parameterize a test in a nice way):
$ diff -ur test/commands/fifo-read-ranges{,-scroll}
-edit -fifo fifo *fifo*
+edit -fifo fifo -scroll *fifo*
$ diff -ur test/commands/fifo-read-ranges-noeol{,-scroll}
-edit -fifo fifo *fifo*
+edit -fifo fifo -scroll *fifo*
[^1]: unless the very last character is a fake newline (except for
the first read, to not scroll) which we already don't report.
|
|
As reported in [1], when reading from a fifo a buffer that is not
newline-terminated, we add a newline automatically, and include that
in the range reported to BufReadFifo.
I'm not 100% sure if this is really wrong but since the typical
consumer of BufReadFifo does something like
select %val{hook_param}
exec |grep foo<ret>
it seems safe to remove this newline; doing so means that
1. "grep foo" will no longer see a newline in stdin, which
seems good.
2. Kakoune will strip a trailing newline from the command output,
since the input didn't have one. So the input is the same.
Let's remove any artificially-added newline from the hook parameter.
[1]: https://lists.sr.ht/~mawww/kakoune/%3CZzXjfXnETd2gbeXa@thristian.org%3E
|
|
On the first read from a nonscrolling fifo, we insert after the buffer
contents (which is just "\n"), and later delete the redundant newline
(582c3c56b (Do not add trailing newline to non-scrolling fifo buffers,
2024-01-28)). This deletion invalidates inserted ranges passed to
BufReadFifo. Fix that.
The test uses another fifo to pass ranges. I guess it could use
"ui_out -until" as well but this seems simpler. The test script needs
a fd but 3 and 4 are already taken so use 5. I didn't find a portable
way to check if it's already taken.
Fixes #5255
|
|
|
|
Ensure perl exists for git blame tests, replace timing sensitive
`ui_out -ignore ...` with `ui_out -until '...'`
|
|
This has been failing on FreeBSD sourcehut CI, for example
https://builds.sr.ht/~mawww/job/1151241
|
|
Internally, all lines have a trailing "\n".
Buffers created empty (like fifo buffers) start with a single line.
When reading data into fifo buffers, we insert *before* the last line's
trailing newline ("last newline"). This enables autoscrolling (enabled
with "edit -scroll") as long as the cursor is on the last newline.
When autoscrolling is disabled, we have a special case to insert
*after* the last newline. This means that a cursor on that newline
won't be moved. Then we transplant the newline character from the
beginning to the end of the buffer. This special case happens only on
the very first fifo read; on subsequent reads, the cursor at position
1.1 will not be moved anway because insertions happen below 1.1.
Since we always insert (effectively) before the last newline, fifo
buffers have a trailing empty line.
For autoscrolling buffers this seems correct; it gives users an
obvious way to toggle autoscrolling.
For non-scrolling buffers the newline is redundant. Remove it.
This requires keeping track of whether the last newline comes from
the fifo, or was added by us. The shortest fix I could find
is to always append to the buffer if not scrolling, and then delete
the added newline character if applicable.
m_buffer.insert(m_scroll ? pos : m_buffer.next(pos), StringView(data, data+count));
if (not m_scroll and not m_had_trailing_newline)
m_buffer.erase(pos, m_buffer.next(pos));
maybe that's the best fix overall; but erasing at the end seems better
than erasing in the middle, so do that whenever possible.
Reported in https://lists.sr.ht/~mawww/kakoune/%3CZbTK7qit9nzvrMkx@gmail.com%3E
|
|
When "edit -fifo" reads data without a trailing newline, the fifo
buffer will not have a trailing blank line. But if there is a trailing
newline, we will get a trailing blank line. This is weird because the
trailing blank line exists for scrolling, it should not be determined
by the data read.
Add a test case to demonstrates the inconsistency which is fixed by
the next patch.
|