From 560e3631ec57d34c679e6b0faec1e0efdd18d915 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Sat, 10 Aug 2024 11:26:26 +1000 Subject: Move debug utils to debug.hh/debug.cc Lots of code includes buffer_utils.hh just for write_to_debug_buffer which pulls many unnecessary dependencies. Reorganise to reduce compile times. --- src/buffer_utils.cc | 30 ------------------------------ 1 file changed, 30 deletions(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 6a66b000..28b2de05 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -271,36 +271,6 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll return buffer; } -void write_to_debug_buffer(StringView str) -{ - if (not BufferManager::has_instance()) - { - write(2, str); - write(2, "\n"); - return; - } - - constexpr StringView debug_buffer_name = "*debug*"; - // Try to ensure we keep an empty line at the end of the debug buffer - // where the user can put its cursor to scroll with new messages - const bool eol_back = not str.empty() and str.back() == '\n'; - if (Buffer* buffer = BufferManager::instance().get_buffer_ifp(debug_buffer_name)) - { - buffer->flags() &= ~Buffer::Flags::ReadOnly; - auto restore = on_scope_end([buffer] { buffer->flags() |= Buffer::Flags::ReadOnly; }); - - buffer->insert(buffer->back_coord(), eol_back ? str : str + "\n"); - } - else - { - String line = str + (eol_back ? "\n" : "\n\n"); - create_buffer_from_string( - debug_buffer_name.str(), Buffer::Flags::NoUndo | Buffer::Flags::Debug | Buffer::Flags::ReadOnly, - line); - } -} - - auto to_string(Buffer::HistoryId id) { using Result = decltype(to_string(size_t{})); -- cgit v1.2.3 From 5d378fa3a5139ca7c075ddf1a533856f53943f73 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 14 Nov 2024 11:38:22 +0100 Subject: Fix overlapping range passed to BufReadFifo in non-scrolling fifos 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 --- src/buffer_utils.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 28b2de05..9a46504d 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -1,6 +1,7 @@ #include "buffer_utils.hh" #include "buffer_manager.hh" +#include "coord.hh" #include "event_manager.hh" #include "file.hh" #include "selection.hh" @@ -214,7 +215,7 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll bool closed = false; size_t loop = 0; char data[buffer_size]; - BufferCoord insert_coord = m_buffer.back_coord(); + Optional insert_begin; const int fifo = fd(); { @@ -235,13 +236,19 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll if (not m_scroll and (is_first or m_had_trailing_newline)) pos = m_buffer.next(pos); - pos = m_buffer.insert(pos, StringView(data, data+count)).end; + auto inserted_range = m_buffer.insert(pos, StringView(data, data+count)); + if (not insert_begin) + insert_begin = inserted_range.begin; + pos = inserted_range.end; bool have_trailing_newline = (data[count-1] == '\n'); if (not m_scroll) { if (is_first) + { m_buffer.erase({0,0}, m_buffer.next({0,0})); + --insert_begin->line; + } else if (not m_had_trailing_newline and have_trailing_newline) m_buffer.erase(m_buffer.prev(pos), pos); } @@ -250,10 +257,10 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll while (++loop < max_loop and fd_readable(fifo)); } - if (insert_coord != m_buffer.back_coord()) + if (insert_begin) m_buffer.run_hook_in_own_context( Hook::BufReadFifo, - selection_to_string(ColumnType::Byte, m_buffer, {insert_coord, m_buffer.back_coord()})); + selection_to_string(ColumnType::Byte, m_buffer, {*insert_begin, m_buffer.back_coord()})); if (closed) m_buffer.values().erase(fifo_watcher_id); // will delete this -- cgit v1.2.3 From 658915086fc4883c45f1af98f1b37a01822e9feb Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 23 Nov 2024 10:54:35 +0100 Subject: Fix BufReadFifo overlapping range on partial line 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 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 --- src/buffer_utils.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 9a46504d..5e55036c 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -258,9 +258,12 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll } if (insert_begin) + { + auto insert_back = m_had_trailing_newline ? m_buffer.back_coord() : m_buffer.prev(m_buffer.back_coord()); m_buffer.run_hook_in_own_context( Hook::BufReadFifo, - selection_to_string(ColumnType::Byte, m_buffer, {*insert_begin, m_buffer.back_coord()})); + selection_to_string(ColumnType::Byte, m_buffer, {*insert_begin, insert_back})); + } if (closed) m_buffer.values().erase(fifo_watcher_id); // will delete this -- cgit v1.2.3 From c3b01a3c9caf030a8c6f6dca56688aca57a9248e Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Wed, 27 Nov 2024 11:13:35 +0100 Subject: Add back option to scroll in stdin buffers Commit 582c3c56b (Do not add trailing newline to non-scrolling fifo buffers, 2024-01-28) completely forgot about stdin buffers, breaking the ability to make them scrolling via "ge". For example while sleep 1; do seq $LINES date done | kak Let's fix that by adding the trailing newline back for stdin buffers. Unlike "edit -scroll", don't scroll until the user explicitly moves the cursor. --- src/buffer_utils.cc | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index 5e55036c..fc87fea5 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -166,7 +166,7 @@ void reload_file_buffer(Buffer& buffer) buffer.flags() &= ~Buffer::Flags::New; } -Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll) +Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, AutoScroll scroll) { static ValueId fifo_watcher_id = get_free_value_id(); @@ -186,7 +186,7 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll struct FifoWatcher : FDWatcher { - FifoWatcher(int fd, Buffer& buffer, bool scroll) + FifoWatcher(int fd, Buffer& buffer, AutoScroll scroll) : FDWatcher(fd, FdEvents::Read, EventMode::Normal, [](FDWatcher& watcher, FdEvents, EventMode mode) { if (mode == EventMode::Normal) @@ -233,7 +233,8 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll auto pos = m_buffer.back_coord(); const bool is_first = pos == BufferCoord{0,0}; - if (not m_scroll and (is_first or m_had_trailing_newline)) + if ((m_scroll == AutoScroll::No and (is_first or m_had_trailing_newline)) + or (m_scroll == AutoScroll::NotInitially and is_first)) pos = m_buffer.next(pos); auto inserted_range = m_buffer.insert(pos, StringView(data, data+count)); @@ -242,14 +243,17 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll pos = inserted_range.end; bool have_trailing_newline = (data[count-1] == '\n'); - if (not m_scroll) + if (m_scroll != AutoScroll::Yes) { if (is_first) { m_buffer.erase({0,0}, m_buffer.next({0,0})); --insert_begin->line; + if (m_scroll == AutoScroll::NotInitially) + m_buffer.insert(m_buffer.end_coord(), "\n"); } - else if (not m_had_trailing_newline and have_trailing_newline) + else if (m_scroll == AutoScroll::No and + not m_had_trailing_newline and have_trailing_newline) m_buffer.erase(m_buffer.prev(pos), pos); } m_had_trailing_newline = have_trailing_newline; @@ -270,7 +274,7 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, bool scroll } Buffer& m_buffer; - bool m_scroll; + AutoScroll m_scroll; bool m_had_trailing_newline = false; }; -- cgit v1.2.3 From 316bca9d6225007e32e89053dc3b8bd221d40b50 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 28 Nov 2024 13:24:47 +0100 Subject: Scrolling BufReadFifo to not not report final empty line 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. --- src/buffer_utils.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index fc87fea5..f311b4c6 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -263,7 +263,8 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, AutoScroll if (insert_begin) { - auto insert_back = m_had_trailing_newline ? m_buffer.back_coord() : m_buffer.prev(m_buffer.back_coord()); + auto insert_back = (m_had_trailing_newline and m_scroll == AutoScroll::No) + ? m_buffer.back_coord() : m_buffer.prev(m_buffer.back_coord()); m_buffer.run_hook_in_own_context( Hook::BufReadFifo, selection_to_string(ColumnType::Byte, m_buffer, {*insert_begin, insert_back})); -- cgit v1.2.3 From 6f29950e913a05bfad8baf1f515e36cf15dd2bb2 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 28 Nov 2024 13:24:48 +0100 Subject: Fix extra newline inserted into stdin buffers Commit c3b01a3c9 (Add back option to scroll in stdin buffers, 2024-11-27) missed the case where the initial read from stdin had no trailing newline: for i in $(seq 50); do printf .; sleep .1; done | kak After the first read, we transplant the initial newline to end. This creates an extra newline because we already added a fake newline to uphold buffer invariants. Fix that. --- src/buffer_utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/buffer_utils.cc') diff --git a/src/buffer_utils.cc b/src/buffer_utils.cc index f311b4c6..a4fb639b 100644 --- a/src/buffer_utils.cc +++ b/src/buffer_utils.cc @@ -249,7 +249,7 @@ Buffer* create_fifo_buffer(String name, int fd, Buffer::Flags flags, AutoScroll { m_buffer.erase({0,0}, m_buffer.next({0,0})); --insert_begin->line; - if (m_scroll == AutoScroll::NotInitially) + if (m_scroll == AutoScroll::NotInitially and have_trailing_newline) m_buffer.insert(m_buffer.end_coord(), "\n"); } else if (m_scroll == AutoScroll::No and -- cgit v1.2.3