ensure cursor is visible after user input except if the command
implementation opted-out. Hooks and timers should not enforce
visible cursor.
PageUp/PageDown and `<c-f>` / `<c-b>` commands still move the cursor
as this seemed a desired behaviour.
Ensure we ignore SIGHUP once the TerminalUI is gone as it will be
sent again on fork, fix the parent process terminating due to trying
to write to stdout after it was closed.
Fixes#4960
This is currently broken on various corner cases and breaks the
"master branch should be good for day to day work" implicit rule,
ongoing work to stabilize this feature will take place on the
no-cursor-move-on-scroll branch until its deemed ready.
This reverts commit 1e38045d70.
Closes#4963
Kakoune now does not touch cursors when scrolling. It checks
if either the buffer or selections has been modified since
last redraw.
Fixes#4124Fixes#2844
RegionsHighlighter::create_region() validates the highlighter type argument
but RegionsHighlighter::create_default_region() assumes it is correct,
segfaulting from dereferencing a null pointer if the given type isn't in
the highlighter registry HashMap.
@PJungkamp reported this in https://github.com/mawww/kakoune/issues/4959
with a simple recipe to reproduce:
:add-highlighter shared/test regions
:add-highlighter shared/test/ default-region invalid highlighter
:add-highlighter window/test ref test
Validate the type argument in RegionsHighlighter::create_default_region()
in the same way as RegionsHighlighter::create_region().
The current exponential behaviour does not seem that useful, it seems
more predictible that pressing `+` twice would end up with 3 copies
of the original selections instead of 4.
Fixes#4533
Commits e49c0fb04 (unmap: fail if the mapping is currently executing,
2023-05-14) 42be0057a (map: fail if key is currently executing,
2023-06-24) fixed potential use-after-free issues. By doing so,
it broke configurations that in practice have not triggered any
crashes [1] [2].
For example with,
set -remove global autocomplete insert
hook global InsertCompletionShow .* %{
map window insert <esc> <c-o>
}
hook global InsertCompletionHide .* %{
unmap window insert <esc> <c-o>
}
The execution of the <esc> mapping triggers InsertCompletionHide fails
at unmapping. This seems legit and I don't see an obvious alternative
way to write it (InsertIdle would not be correct though it would work
in practice).
Fix the regression by allowing map and unmap again while keeping the
mappings alive until they have finished executing.
Applying map/unmap immediately seems like the most obvious semantics.
Alternatively, we could apply them in between key presses.
[1]: <https://github.com/kak-lsp/kak-lsp/issues/689>
[2]: <https://github.com/alexherbo2/auto-pairs.kak/issues/60>
If during execution of a mapping, that same mapping is replaced,
there is undefined behavior because we destroy a mapping that we are
still iterating over.
I have been using this mapping inside my kakrc to re-source the kakrc.
map global user s %{:source "%val{config}/kakrc"<ret>} -docstring 'source "%val{config}/kakrc"'
Now <space>s happens to not trigger undefined behavior because the
mapping stays the same.
However it triggers an assertion added by Commit e49c0fb04 (unmap:
fail if the mapping is currently executing, 2023-05-14), specifically
the destructor of ScopedSetBool that guards mapping execution.
Fix these by banning map of a key that is executing, just like we
did for unmap.
Alternative solution: we could allow mapping (and even unmapping)
keys at any time and keep them alive by moving them into a trash can,
like we do for clients and others.
When a line only contains non-range atoms we can end-up accessing
past the end atom.
Add a test that shows the issue when run with valgrind, it is
unfortunately quite hard to trigger a crash because the invalidly
accessed byte usually leads to the correct code path being taken
(when != DisplayAtom::Range) so we have only 1 in 255 chance of
triggerring a crash.
Fixes#4927
In some cases such as with folding we can end-up with regions
not having any atoms to highlight which can trigger a crash as
we assume display buffers not to be empty
Fixes#4926
My terminal allows to map <c-[> and <esc> independently. I like
to use <c-[> as escape key so I have this mapping:
map global prompt <c-[> <esc>
Unfortunately, this is not equivalent to <esc>. Since mappings are
run with history disabled, <c-[> will not add the command to the
prompt history.
So disabling command history inside mappings is wrong in case the
command prompt was created before mapping execution. The behavior
should be: "a prompt that is both created and closed inside a
noninteractive context does not add to prompt history", where
"noninteractive" means inside a mapping, hook, command, execute-keys
or evaluate-commands.
Implement this behavior, it should better meet user expectations.
Scripts can always use "set-register" to add to history.
Here are my test cases:
1. Basic regression test (needs above mapping):
:nop should be added to history<c-[>
---
2. Create the prompt in a noninteractive context:
:exec %{:}
now we're back in the interactive context, so we can type:
nop should be added to history<ret>
---
3. To check if it works for nested prompts, first set up this mapping.
map global prompt <c-j> '<a-semicolon>:nop should NOT be added to history<ret>'
map global prompt <c-h> '<a-semicolon>:nop should be added to history first'
Then type
:nop should be added to history second<c-j><c-h><ret><ret>
the inner command run by <c-j> should not be added to history because
it only existed in a noninteractive context.
---
See also the discussion https://github.com/mawww/kakoune/pull/4692
We could automate the tests if we had a test setup that allowed
feeding interactive key input into Kakoune instead of using
"execute-commands". Some projects use tmux, or maybe we can mock
the terminal.
The commit after next will fix a bug where we wrongly disable prompt
history in some scenarios. The root cause is that life span of
"disable_history" does not model when we actually want to disable
history.
Let's rename the state variable to "noninteractive". It's set whenever
we are executing a hook, mapping or command.
Note that it's also active inside ":prompt"'s callback, which doesn't
play well with the new name :(
This switch makes show-matching fallback to the character preceeding
the cursor if the character under the cursor is not a matching
character, which should make show-matching more useful in insert mode.
Cache get fully invalidated whenever the regions change, so there
should be no risk of referencing a removed region, and this removes
one hash map lookup for every region in the displayed buffer range.
Range atoms should always appear in order, so we can iterate a single
time through the display lines and display atoms while applying
hightlighters to regions
