| Age | Commit message (Collapse) | Author |
|---|
|
Commit ba41928aa (Remove spurious newline when | replaces at buffer end,
2024-11-28) also tried to fix a "DiffOp::Add" code path. Unless I'm
misremembering, I had run into a crash and extracted this unit test.
When apply_diff() decides to erase the last line, and then insert something
at the end, buffer changes might look like:
{.type=Erase, .begin={2, 0}, .end={3, 0}}
{.type=Insert, .begin={1, 5}, .end={5, 0}}
The "{1, 5}" is caused by "pos = buffer.prev(pos);" added by the
aforementioned commit. It's problematic because it breaks the caller's
"ForwardChangesTracker" invariant: the previous Erase leaves the cursor at
{2, 0}, so the insert cannot start before that.
Additionally, the inserted range returned by apply_diff() would be
"{ {2, 0}, {4, 0} }" whose end position is weirdly different from the Insert's
end position. It questionable that we are passing on this weird state.
The "pos = buffer.prev(pos);" statement was added so we actually delete the
current final newline. We still leave "tried_to_erase_final_newline" set,
meaning we'll also delete the the final newline after we're done.
But deleting the "current final newline" the way we do it is confusing,
because we do it at every step DiffOp::Add step. This would blow up if
there are multiple successive contiguous DiffOp::Add events. I doubt this
ever happens but it's still confusing.
Remove this logic, and restore historical behavior in case we append at the
buffer end. This does not break the system test added by ba41928aa because
in that case, we
1. first delete the entire buffer, setting pos={0, 0}, but also restoring
the EOL invariant, so the end position would be {0, 1}.
2. then we insert at pos={0, 0}, and since "buffer.is_end(pos)" is false we
don't run into this bad code path.
While at it add an assertion to clarify that we already assume that the
diff algorithm never gives us empty DiffOp::Keep, and always monotonically
increasing positions, except for the very special case where we deleted
until buffer end, which should already be handled.
The unit test is not perfect because it depends on the current behavior of
the diff algorithm.
|
|
`kak -n -E 'hook global WinCreate .* %{ delete-buffer }'` was crashing
because we would delete the buffer during window construction, which
would not be able to delete the window as it was not fully constructed
and registered yet. This led to a window referencing a deleted buffer.
Fixing this by deleting the window later on failed because we can enter
an infinite loop where we constantly create a new *scratch* buffer, then
a window to display it, which deletes that buffer.
Make it an error to try to delete a buffer while a new window is
being setup by adding a Locked flag to buffers and checking that in
BufferManager::delete_buffer
Fixes #5311
|
|
The hook is manually triggred at the end of registration, by
disabling it we avoid the hook being potentially called multiple
times due to interaction with other hooks.
Fixes #5324
|
|
file.cc/hh should not know about Context, Buffer, etc... It should
be a pretty low level set of helper functions. Move buffer related
functions to buffer_utils and extract busy indicators to callers.
|
|
This got pushed by accident
This reverts commit 2856b99e0914cc7a659977f2b33308cb5b4c9bb7.
|
|
|
|
|
|
This makes BufferIterator smaller and trivially move/copyable
|
|
The extra indirection of going through the buffer can be costly as
the compiler does not know the buffer is not supposed to be mutated
during iteration, so it has to actually reload the values which adds
memory accesses in the Buffer instance which can be costly in say
regex searches where memory access tends to dominate performance.
Storing this in the BufferIterator lets the compiler put this
info in registers and not reload it.
|
|
|
|
Moving across history moved to <c-j>/<c-k> to keep <a-u>/<a-U>
for selection undo/redo
This reverts commit e0d33f51b36c9f0be7ae2467dab455d211bbf561.
|
|
Whenever a new history node is committed after some undo steps, instead
of creating a new branch in the undo graph, we first append the inverse
modifications starting from the end of the undo list up to the current
position before adding the new node.
For example let's assume that the undo history is A-B-C, that a single undo
has been done (bringing us to state B) and that a new change D is committed.
Instead of creating a new branch starting at B, we add the inverse of C
(noted ^C) at the end, and D afterwards. This results in the undo history
A-B-C-^C-D. Since C-^C collapses to a null change, this is equivalent to
A-B-D but without having lost the C branch of the history.
If a new change is committed while no undo has been done, the new history
node is simply appended to the list, as was the case previously.
This results in a simplification of the user interaction, as two bindings
are now sufficient to walk the entire undo history, as opposed to needing
extra bindings to switch branches whenever they occur.
The <a-u> and <a-U> bindings are now free.
It also simplifies the implementation, as the graph traversal and
branching code are not needed anymore. The parent and child of a node are
now respectively the previous and the next elements in the list, so there
is no need to store their ID as part of the node.
Only the committing of an undo group is slightly more complex, as inverse
history nodes need to be added depending on the current position in the
undo list.
The following article was the initial motivation for this change:
https://github.com/zaboople/klonk/blob/master/TheGURQ.md
|
|
|
|
Comparing iterators between buffers should never happen, and the
only place we did was with default constructed BufferIterator which
we replace by casting the iterator to bool.
This should improve performance on iterator heavy code.
|
|
|
|
|
|
The real technical limit is with lines bigger than 2 GiB and buffers
with more than 2 Gi lines, refactor buffer loading to make it possible
to load those files.
Fix an overflow with the hash_data function at the same time
|
|
|
|
|
|
Do not access Buffer::m_changes to find the inserted range, return
it directly from Buffer::insert and Buffer::replace. This fixes a
wrong behaviour where replacing at eof would lose the selected end
of line (as the implementation does not actually replace that end
of line)
|
|
|
|
Instead of triggering a reload event when the timestamp of a buffer's
underlying file changes, do so when its contents are actually modified.
|
|
Fixes #3233.
|
|
|
|
|
|
|
|
BufferIterators are large-ish, and need to check the buffer pointer
on comparison. Checking against a coord is just a 64 bit comparison.
|
|
It seems unlikely this would give performance gain, as buffer
lines are always accessed when we read that field, leading to
all the necessary data already being in memory. Removing it
reduces the size of a BufferIterator, which are already pretty
hefty objects.
|
|
Hooks are now an enum class instead of passing strings around.
|
|
Avoids warning about non virtual destructor calls on them,
as they have a vtable due to OptionManagerWatcher.
|
|
Store the undo tree as an array of undo nodes, instead of as a
pointer based tree.
|
|
|
|
The debug buffer is a bit special as lots of events might mutate it,
permitting it to be modified leads to some buggy behaviour:
For example, `pipe` uses a ForwardChangeTracker to track buffer
changes, but when applied on a debug buffer with the profile flag
on, each shell execution will trigger an additional modification
of the buffer while applying the changes, leading to an assertion
failing as changes might not be happening in a forward way anymore.
Trying to modify a debug buffer will now raise an error immediatly.
|
|
|
|
|
|
|
|
The need to have the array size in the return type was redundant with
the actual list of elements.
|
|
We were preserving the history in that case, so on fifo buffers
(that set the NoUndo flag until the fifo is closed), we still had
the history from the "previous life" of the buffer, leading crashes
when trying to apply it.
Fixes #1518
|
|
|
|
Until now, buffer had multiple recognized end coordinates, either
{ line_count, 0 } or { line_count - 1, line[line_count - 1].length }.
Now the only correct end coord is { line_count, 0 }, removing the need
for various special cases.
|
|
|
|
|
|
|
|
We should never destruct anything through an OptionManagerWatcher
pointer, so having all those destructor virtual makes no sense.
|
|
|
|
|
|
|
|
This way we dont depend on knowing the base template to enable bit ops
on an enum type.
|
|
Introduce Meta::Type<T> to store a type as value, and pass it
around, migrate enum_desc and option_type_name to this.
|
|
Fixes #1228
|
