| Age | Commit message (Collapse) | Author |
|---|
|
computing the indentation is surprisingly costly (close to 8% of the
whole test framework runtime), and having the full path to easily copy
paste is handy instead of recreating it from the printed tree.
|
|
Consider atom boundaries as word boundaries, which should be correct
du to passes ordering.
Fixes #5350
|
|
If the current buffer is locked, it means we are in the middle of
creating the client or already changing the buffer, error out in this
case instead of getting into an inconsistent state.
Fixes #5338
|
|
My
kak -e "exec %{%ca<ret>b<esc>%|printf '\n\n'<ret>}"
adds a spurious third line.
When we replace up to the end everything, we keep around a single
newline to uphold the buffer invariant, but that newline
Commit de1433d30 (Avoid the spurious newline insertion when replacing
at end of buffer, 2016-03-16) fixed an issue for most commands that
replace until the buffer end.
A similar issue exists for the "|" command. It triggers in fewer
cases because the replacement is implemented by applying edits
computed by the diff algorithm. It does trigger when "|" replaces
the entire buffer.
Fix that by erasing the spurious newline.
Alternatively, we could allow the diff steps of kind "remove" to
delete the entire buffer, and only restore the invariant after the
whole diff is applied. This should work because the one-past-end
position is valid for Buffer::insert() even if the buffer is empty. It
is not valid for Buffer::erase() or Buffer::advance() where count>0
but if we do that when we're already at the buffer end, that is
probably a bug in the diff. I tried this but ran into a assertion
in ForwardChangesTracker (kak_assert(change.begin >= cur_pos)).
Alternatively, we could change the diff algorithm to always insert
before deleting.
I encountered this issue when using ansi-enable on a fifo buffer.
Specifically, the first BufReadFifo hook would replace the entire
inserted text but leave around a spurious newline.
|
|
This test uses ui_out and ui_in to coordinate events.
This is brittle[1] because ui_out behavior depends on timing.
Since this test doesn't really care about intermediate UI state,
express the sequence using BufCloseFifo instead.
This hits another issue: inside git blame-jump's BufCloseFifo, we
run git blame, which runs another "edit -fifo .. *git*". A special
aspect of fifo buffers is that any existing *git* buffer will be
reused instead of being recreated[2]. After BufCloseFifo, the fifo
watcher destructor will reset the fifo flag, even if BufCloseFifo
has recreated the fifo buffer. This breaks invariants and causes
the next fifo watcher destructor do fail its assertion.
Let's not reset fifo flags in this case. This should be safe because
it's the very last thing the fifo watcher does, so it's okay if
another one is active now.
Alternatively we could reject this kind of recursion, or implement
it in a different way (using ScopedSetBool for the flags).
Reported-by: Nico Sonack <nsonack@herrhotzenplotz.de>
[1]: https://lists.sr.ht/~mawww/kakoune/%3C20241210100417.1150697-1-aclopte@gmail.com%3E
[2]: This removes the need to use delete-buffer which also ensures that
the buffer remains visible in any client it's already shown.
|
|
FifoWatcher::read_fifo() deletes the fifo watcher in
m_buffer.values().erase(fifo_watcher_id); // will delete this
which calls: HashMap::unordered_remove()
constexpr_swap(m_items[index], m_items.back());
destructor called here --> m_items.pop_back();
m_index.remove(hash, index);
So hash map invariants (of buffer.values()) are broken, when calling
~FifoWatcher which fires BufCloseFifo hooks. Things blow up if those
hooks access buffer.values() such as by accessing cached highlighters
to redraw the buffer. A shell call with a long sleep in the client
context seems to trigger this.
Fix this by destroying removed map items only at the end of
HashMap::remove(), when invariants are restored. Alternatively, we
could introduce a fifo_trash container; I haven't explored that.
|
|
Replaced ranges will count towards the wrapping column but will not
be split.
Fixes #4883
|
|
Move the whole responsibility of making the cursor visible to the
window, removing cursor_pos from the display setup and resolving
the cursor location by finding it in the display buffer afterwards.
This simplifies hightlighters' do_compute_display_setup as they do
not need to compute the cursor location. Highlighting may run on
more lines than necessary after this change but this should be a
minor performance hit.
|
|
|
|
assert_eq got renamed to check_json_eq but the fact this was used by
tests in their script file was missed, renamed it back to assert_eq
as this function is not necessarily used for json.
|
|
|
|
If git is present we can rely on git-diff word support to get a much
easier to understand diff.
|
|
Without doing this, replace with multiple selection at buffer end
breaks as the first selection will replace the end-of-line with a
non-eol character, then a new eol will automatically be added which
will put remaining selections past the end of the buffer.
Fixes #5316
|
|
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
|
|
We use linewise paste if any of the pasted register elements has a trailing
newline. As shown in the previous patch, this leads to awkward situations
where insertion positions from <a-P> are not monotonically increasing.
This complicated behavior is questionable. The root cause is that linewise
paste may insert some strings that don't have a trailing newline. This leads
to weird cases of GIGO, especially when using <a-P> when the last element
is missing a trailing newline.
Simplify the implementation by not activating linewise paste in that case.
Instead of special-casing the last element, try to simplify the behavior
further by using line-wise paste only if *all* elements are properly
terminated. I can't think of a real-world case where the existing behavior
would be desired.
A regression test shows a change in behavior. Might be less weird now.
The added test case is a copy except it uses "P" instead of "p" (the extra
newline is because "d" deletes the last newline in the buffer which gets
added back implicitly).
(I think the list of register elements is never empty, so this should never
cause a trivial linewise paste. Even if so, that would be fine because
inserting nothing is a nop either way.)
Fixes #5312
|
|
Given a buffer with two selections
%(a)b%(c)
Consider <a-P>, which inserts all elements of the dquote register, joined
to a scalar string. This insertion happens once before each selection.
Let the dquote register contain "\n" and "\nd". Since the first element ends
in a newline, we enable linewise paste mode.
The insertion at the first selection results in:
contents: \n\nd%(a)b%(c)
timestamps: 11111000000000
On top of that, the insertion at the second selection would result in:
contents: \n\n\n\ndd%(a)b%(c)
timestamps: 1111222221000000000
Observe that the second insertion actually takes place inside the text added
by the first insertion.
This is because we are inserting before the current line, and the first
insertion does not end in a newline.
This breaks the forward change tracker's assumption that each change takes
place after the previous one, causing an assertion failure. We use this
data structure to translate from old to new coordinates. Specifically,
after the first insertion, the selection around %(c) which is initially
1.3,1.3 needs to be updated to 3.4,3.4.
Work around this by instantiating a new ForwardChangesTracker after each step.
This is very ugly because it's quadratic, and because it doesn't change the
fact that the second insertion is made inside the first one. I think we
can revert the workaround added by this patch, see the next patch.
Closes #5312
|
|
The <?> and <a-?> commands drop selections where the search wrapped.
If a selection is dropped this way, we adjust the new main index.
If the very first selection is dropped this way, *and* it happens to
be the main index, "--main_index" overflows and chaos ensues.
Fix this by saturating main index at zero. This seems more intuitive
than wrapping around ("% new_sels.size()").
This issue only happens to <a-?>, not <?>. When <?> wraps when starting
from the first selection, it necessarily also wraps after all other
selections. (Based on this insight, I started drafting an optimization
to skip searches whose results would be discarded anyway because they
will definitely wrap. Not sure if it's worth it, since it applies
only to the rare edge case of <?>/<a-?> with multiple selections.)
Fixes #5314
|
|
Add a test covering this branch in extend_to_next_matches():
else if (new_sels.size() <= main_index)
--main_index;
Specifically, this test would fail if the "<=" were to be replaced with "<".
|
|
sel1 == sel2 was using BasicSelection::operator==, ignoring captures,
which led to selection history deciding that selection did not change
in some cases where only the captures had been updated and for the
new selection not to be properly applied, leading to missing capture.
|
|
Commit e3122ab2c (Refactor prompt history handling, 2023-07-05) was a nice
simplification but it breaks a rare edge case. It suppresses history
recording if all keys the prompt receives were synthesized. That's not
quite the right criteria:
it means that if prompt is created, edited and and executed, all via mapped
keys, we fail to add to history.
The criteria should rather be something like "if all keys the prompt receives
came from synthesized events".
Make it so. This allows us to get rid of the "noninteractive" nested bool
that was only used for disabling history.
|
|
Surprisingly, these two commands show different commit times:
git blame README.asciidoc
kak -e 'git blame' README.asciidoc
This is because git shows times as of the original time zone (of the
author/committer). Our blame integration uses the current local time.
- blame-jump displays the date in the status line of a git-show buffer
This date may be inconsistent with the buffer's "Date:" header,
so this seems surprising. Fix that. This fixes a test in some
time zones.
- Unlike "git blame", our ":git blame" does not display time zone
info by default. So, the conversion to localtime might make sense.
I don't really have an opinion on this. Change it too I guess,
since the current behavior might not have been intended.
Fixes #5285
|
|
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
|
|
Commit 8c49c8ee4 (Highlight all atoms for the line regardless of
has_buffer_range, 2024-11-04) intentionally changes the behavior
to also highlight outside the buffer, painting over number-lines.
Let's use a final face as suggested to restore the test's previous
behavior, fixing the failure.
|
|
"ui_out -until-grep" invocations used to redirect the output from
grep, usually to /dev/null. Looks like the intention of 60fcc3443
(Change ui_out -until-grep to check for equality the next argument,
2024-11-02) was that grep no longer needs to print output, since we
can assert instead which is mildly better.
Unfortunately there are two places where -until-grep might not be
powerful enough, which is why we capture the output and check it
in the test script. For one of them we can use -until-grep with a
small change. Not yet sure about the other one. Let's try to use
eval instead, so we can silence the output.
I realize -until-eval is not great (the nested quoting is ugly)
but I guess it's better than the status quo.
Alternatively, we could print output only if the [expected] argument
is not given, and add >/dev/null to the other invocations.
|
|
Tests print a confusing error
./run: line 214: shift: shift count out of range
This is because ui_out -until-grep now shifts twice which conflicts
with the weird shift at the end of ui_out.
It also has a loop, making it possible to do something like "ui_out
foo bar baz" to expect three different messages. We don't use this
feature anywhere, and it seems just super surprising, given that
ui_out now consumes between 1 and 3 arguments each time. Let's get
rid of the loop, and always run shift right next to where we actually
use the argument. This should make things less brittle.
|
|
This changes the behaviour with say line numbers and other line
flags, but can be opted out by using final faces
Fixes #5253
|
|
|
|
Instead of outputing the matching output, either ignore the output
(if not extra argument is given) or match it.
Usage is: ui_out -until-grep <pattern> [<expected>]
|
|
|
|
Share most logic with shell-script-candidates. Now that we do not
block we can run the completion script implicitely instead of waiting
for an explicit completion request with <tab>.
Fixes #5245
|
|
Before performing the insertion, InsertCompleter::insert calls
try_accept() to accept any selected completion candidate. If there
is one, we fire InsertCompletionHide. If that one modifies the register
used by <c-r>, the inserted StringViews will be dangling.
Fix this by running try_insert first, and read from the register later.
Note that we call try_accept() twice but that's fine.
It would probably make more sense to copy the register before calling
insert() but I don't think it matters.
Closes #5220
|
|
Add cursor coordinate parameters, and fix encoding to match the
new one.
|
|
|
|
|
|
A common pattern is for info/echo messages to be generated by idle
hooks but the clearing of previous info/echo was done immediately on
normal mode events. This led to flickering of the info box especially
when a hook was repeatidly generating the same info (like moving
a cursor in the same word where the hook reacts to the word under
the cursor).
|
|
Make last insert and macro recording closer together, paving the
way towards moving last insert to a register.
Use a FunctionRef for insert completer key insertion support.
|
|
This fixes an issue with the following hooks:
hook global InsertCompletionShow .* %{ map window insert <tab> <c-n> }
hook global InsertCompletionHide .* %{ unmap window insert <tab> <c-n> }
When repeating a last insert using <tab> to select a completion, it
inserts a <tab> instead of selecting, then the insert completion tries
to erase the inserted text with backspaces but fails to totally do that
as it erases the tab character first.
|
|
We do not typically go into lengthy explanation of the code in the
support scripts. This would have a performance impact (as comments
are not trimmed in advance) and feels out of place.
|
|
The current python filetype module treats a single empty comment line
(typically created by hitting enter twice while in a block comment) as
the end of a block comment, deleting the empty comment and ending
comment prefix copying. This runs contrary to PEP8, which explicitly
allows for paragraphs in block comments, with an empty comment as the
paragraph separator.
This change implements support for using a single empty comment as a
paragraph separator, with two consecutive empty comments being treated
as the end of the block comment; both empty comment lines are deleted
and comment prefix copying is ended.
|
|
|
|
Ensure perl exists for git blame tests, replace timing sensitive
`ui_out -ignore ...` with `ui_out -until '...'`
|
|
ASan shows that we resolve a face spec owned by a freed stack variable.
=================================================================
==2263300==ERROR: AddressSanitizer: stack-use-after-return on address 0x7a9316c33918 at pc 0x633ea421d8ea bp 0x7ffca001e980 sp 0x7ffca001e970
READ of size 8 at 0x7a9316c33918 thread T0
...
#6 0x633ea421d8e9 in Kakoune::FaceRegistry::resolve_spec(Kakoune::FaceSpec const&) const src/face_registry.cc:128
...
Address 0x7a9316c33918 is located in stack of thread T0 at offset 2328 in frame
#0 0x633ea427a095 in operator() src/commands.cc:2267
This frame has 26 object(s):
[32, 36) '<unknown>'
...
[544, 560) 'disable_hooks' (line 2269)
...
[928, 2432) 'local_scope' (line 2271) <== Memory access at offset 2328 is inside this variable
|
|
A recent commit highlights char literals as value (default red)
instead of yellow.
|
|
|
|
tabs <-> space conversion functions did not create a ScopedEdition
leading to uncommited modifications. Fixing this did fix the
interactive error, but that error still existed in non interactive
context so redo now considers there there is no redo child if there
are uncommited modifiations (which is correct as this means we are
currently creating a new leaf in the undo tree)
Fixes #5124
|
|
Only record non-synthetized insertions, removing the need to
re-record on replay and fixing the last replay getting dropped by
macro execution.
Fixes #5122
|
|
|
|
<c-n>/<c-p> handling in insert was always dropping the last key in
the last_insert() vector (in order to replace it with the actual
completion text inserted), this was not valid for synthetized keys
that are not added to that vector in the first place.
Take the opportunity to merge insert completion handling code between
<c-n>/<c-p> and direct menu selection.
Fixes #5120
|
