Given a completer option with two applicable completions
text|select-cmd1|menu-text1
text|select-cmd2|menu-text2
Kakoune will only show one of them, because they will insert the
same text.
Some language servers send completions like this, for example if
two different importable modules provide the same name. This can be
reproduced using intelephense in this PHP file (cursor is %())
<?php
namespace namespace1;
class sometype {}
?>
<?php
namespace namespace2;
class sometype {}
?>
<?php
namespace test;
some%()
?>
Both completions insert "sometype". The import statement will be
added in an InsertCompletionHide hook by kak-lsp (it uses select-cmd
to determine which completion was selected).
To support this use case, refine the duplicate detection to not filter
out completions with different select-cmd values.
Parsing a (non-valid) font with a comma in the name of the base colour
makes Kakoune crash. It is not a valid face, but Kakoune should just
return an error message instead.
Reproducer:
:set-face global foo ,red@,blue
Note the comma "," after the "@". This is not a valid base name, and it
leads to a crash. Let's see what happens.
At the beginning of parse_face(), we have the following code:
auto bg_it = find(facedesc, ',');
auto underline_it = bg_it == facedesc.end() ? bg_it : std::find(bg_it+1, facedesc.end(), ',');
auto attr_it = find(facedesc, '+');
auto base_it = find(facedesc, '@');
[...]
auto colors_end = std::min(attr_it, base_it);
After this:
- bg_it points to ",red@,blue"
- bg_it != facedesc.end(), so we have underline_it pointing to the first
comma after bg_it. This means that underline_it points to ",blue"
- base_it points to "@,blue"
- attr_it points to the end of facedesc (no "+" marker), so colors_end
points to base_it, "@,blue"
Later in the code, just after parsing the foreground and background
colours, we have:
if (underline_it != facedesc.end())
face.underline = parse_color({underline_it+1, colors_end});
When passing {underline_it+1, colors_end} to parse_color(), we pass in
fact iterators pointing to {",blue", "@,blue"}. Because the second one
starts _before_ the first one in the string, this means that the
resulting string is considered to have a _negative_ length.
parse_color() passes the string to str_to_color(), who fails to turn up
the colour, and attempts to throw:
throw runtime_error(format("unable to parse color: '{}'", color));
The variable "color" still has this negative length, and this goes all
the way down to an assert in src/units.hh where we expect that string to
be >= 0, and we crash on the assertion failure.
For similar reasons, we also get a crash if the comma comes after the
marker for the face attributes:
:set-face global foo ,red+,a
To fix both cases, let's add a check to make sure that the underline_it,
marked with a comma, never gets detected as present and pointing after
colors_end, be it "@" or "+".
As pointed out in [1], when insert mode autocomplete is disabled,
<c-n> could be used to activate insert mode completions temporarily
[2]. This regressed in 6f7c5aed (Do not show custom completions when
autocomplete is off, 2022-01-03). Fix this by enabling completions
on <c-n>/<c-p>. This allows us to remove a special case for explicit
completers.
Alternative behavior (future?): make <c-n> toggle completion like
<c-o>. This can be done today, as suggested by Screwtape on IRC:
map global insert <c-n> %{<c-o><c-n><a-;>:toggle-ctrl-n<ret>}
define-command toggle-ctrl-n %{
hook global InsertCompletionShow .* %{ map window insert <c-n> <c-n> }
hook global InsertCompletionHide .* %{ unmap window insert <c-n> <c-n> }
}
[1] https://github.com/mawww/kakoune/pull/4493#issuecomment-1031189823
[2] <c-n> completion only lives for the lifetime of the completion
menu, whereas <c-o> lasts until you exit insert mode. This means
that autocompletion is much more convenient than <c-n> or <c-x>f,
because those require an explicit completion request for each
path component.
The ThreadedRegexVM implementation does not execute split opcodes as
expected: on split the pending thread is pushed on top of the thread
stack, which means that when multiple splits are executed in a row
(such as with a disjunction with 3 or more branches) the last split
target gets on top of the thread stack and gets executed next (when the
thread from the first split target would be the expected one)
Fixing this in the ThreadedRegexVM would have a performance impact as
we would not be able to use a plain stack for current threads, so the
best solution at the moment is to reverse the order of splits generated
by a disjunction.
Fixes#4519
This will unfortunately break some use case which will require
using wrapper scripts to add the necessary newline. It is however
harder to do the contrary, and it makes a lot of other use case
possible, such as checksuming.
Fixes#3669
After a while it seems clear changing this is much more ergonomic
and restoring it with pure config is impractical as we need to map
all lower case keys.
As reported in [1], completions provided by "set global completers
option=my_completion" activate insert mode autocompletion, even when
the autocomplete option does not have the insert mode flag.
This happens because InsertCompleter::on_option_changed() calls
InsertCompleter::setup_ifn(), which shows the completion pager.
Fix this by computing whether the completion pager is enabled;
otherwise we can return early from setup_ifn().
The completion pager is enabled if the autocompletion bit is set,
or if the user has requested explicit completion.
[1]: https://github.com/kak-lsp/kak-lsp/issues/585
In normal mode, the mode line contains "1 sel" or "n sels (k)" when n > 1,
whereas in insert mode, it contains "n sels (k)" even for n == 1. Change
the contents in insert mode to match normal mode.
trim_indent call was incorrect, trim_indent is intended to work
on multi-line strings and trims trailing whitespace as well (could
benefit from a better name).
Fixes#4378
Merge all lookarounds into the same instruction, merge splits, merge
literal ignore case with literal...
Besides reducing the amount of almost duplicated code, this improves
performance by reducing pressure on the (often failing) branch target
prediction for instruction dispatching by moving branches into the
instruction code themselves where they are more likely to be well
predicted.
Only ui type Terminal is intended to be a user interactive session.
If your ui type is not Terminal, don't worry about making
the tty your stdin if fd 0 is not a tty.
This allows json-rpc commands sent via stdin to be acted up rather
than sent to a fifo (which is the default behavior for kakoune).
Does not change the behavior for Terminal ui sessions
Now that Kakoune opts into extended key reporting, <c-i> is correctly
reported and hence needs to be mapped to forward jump.
We still need to keep <tab> mapped to it for legacy terminals.
Should fix#4333
Closing/reopening the read side seems to sometimes lead to
end-of-file not being received, leaving some extra data unexecuted.
The FDWatcher stays disabled during the executing of the fifo
commands so this should not enable any more commands to be executed
until we get back from the CommandManager and reset the FDWatcher
fd to to fifo read end.
Fixes#4410
ARM uses @ as a comment character, so %progbits must be
used in place of @progbits here. This change fixes the
build on armv7 FreeBSD 13.0.
Fixes mawww/kakoune/issues#4385
See also https://bugs.freebsd.org/259434
Cppcheck produces the following warnings:
```
shared_string.hh:27:49: portability: Shifting signed 32-bit value by 31 bits is implementation-defined behaviour
shared_string.hh:27:49: error: Signed integer overflow for expression '1<<31'.
```
Fixes#4340
Cppcheck produces the following warning:
```
keymap_manager.hh:54:37: performance: Function parameter 'user_mode_name' should be passed by const reference.
```
Fixes#4340
Enable it if supported by default, let the user override it with
the existing terminal_synchronized ui option.
This should finalize work discussed on #4317
They are quite different use cases, and this allow moving InsertMode
to input_handler.hh which is what uses it.
This also cleans up the code as we can get rid of get_insert_pos and
rely more on SelectionList::for_each.
This commit prevents `ga` from returning a “no last buffer” error
when the previously displayed buffer was removed.
Since the jumps list keeps track of the order in which buffers were
displayed already, handling arbitrary `delete-buffer`s as well,
cycle through it to implement `ga` instead of storing a pointer.
Note that this commit doesn't take into account buffer flags that
might exclude some buffers from being cycled over by commands.
Fixes#1840
The current implementation allows users to declare mappings in the
`goto` and `view` modes with uppercase letters, but doesn't consider
case to be meaningful.
This quirk is also inconsistent as hitting an unmapped key will quit
the mode menu without complaints, but hitting a key that isn't in it
will work if it has a lowercase mapping equivalent.
Fixes#3976
Just validate if line changed or not. This should avoid flickering
on terminals such as the linux console that eagerly redraw on line
deletions. Unfortunately this means drawing will use more data and
might add a bit of latency on slow links.
Fixes#4317Fixes#4320
kak-lsp uses these faces to mark errors inside the buffer, instead of the Error
face which is much more jarring, and which does not have an associated warning
face. Since the :spell command marks errors inside the buffer, it's also updated
to use this new face.
Adding these faces to Kakoune makes it more likely that colorschemes will
automatically do the right thing when used with kak-lsp, and makes it possible
to use a subtle appearance (like curly underlines) for in-buffer errors while
keeping Kakoune errors bold and jarring as they should be.
Add support for a third color in face definition that controls
the underline and a 'c' attribute for curly underline (that takes
precedence over 'u' if both are specified)
Allow empty colors to mean default, so that `,,red+u` means the
same as `default,default,red+u`
Fixes#4138
Commit 2289f350 ("Remove command parsing Reader and just track a
ParserState") introduced a small regression in parse_percent_token()
because we failed to recognize a token like %val{ as percent-expansion.
I tried to add a test case but a UI test doesn't seem possible, e.g.
kak -ui json -e "exec ':echo %opt{<tab>}<ret>'"
prints: 'exec': option not found: ''
Make String::Data use trivial copy of the short/long union to avoid
unnecessary branching there, inline release() as it can be elided by
the compiler on moved-from Strings.
Display a whitespace in place of the uncovered half of the codepoint.
(I know this is incorrect and we should be considering grapheme clusters
instead of codepoints, but this is a far bigger refactoring and another
can of worms to handle with terminal emulators).
Fixes#4262
Synchronized output does not work well with various terminals
(including the linux console). It should also be unnecessary when
not going through a slow link.
This will eventually be removed if it is not proven to be useful
to some users.
TerminalUI::suspend() is responsible for undoing all Kakoune's modifications to
the terminal state, actually suspending the process, then re-applying all the
modifications after Kakoune wakes back up.
Previously, the "undo" and "reapply" steps for termios settings were both after
the suspend point, so on some platforms they were incorrect when the user
arrived back at the shell prompt.
Now, the termios "undo" step is back before the suspend point, and the undo and
reapply steps should be in exactly reversed order.
Fixes#3488.
In terminals that support it, this sequence causes the terminal to not redraw
*its* output until the application has finished, reducing redraw flickering.
The sequence is defined in:
https://gitlab.com/gnachman/iterm2/-/wikis/synchronized-updates-spec
...and is apparently also supported by kitty and libVTE.
Fixes#3482.
insert blank seems to behave differently between terminals and would
be less efficient because it still has to shift all following characters
(that we will overwrite anyway).
Fixes#3437
Diff against known state and insert/erase relevant lines.
Erase everything first to avoid insertion invalidating lines that
get out of the terminal at bottom.
This should reduce flicker, by avoiding transient states where
info/menu windows are not displayed, and paves the way for proper
diffing of the screen.
Log error to debug buffer and Change the 'waiting for shell' face
to 'Error'.
Update the 'waiting for shell' message when the shell has exited
but Kakoune is still waiting on stdin/stdout/stderr to be closed.
Those fifos are accessible during %sh{...} blocks, the command fifo
executes commands written to it once the write end side is closed
(multiple open/write/close sequences are supported), the response
fifo is a simple helper fifo available to write response back to
the shell process
An example use of this feature is to request some list options
content from without being limited by the environment size:
```
%sh{
echo "echo -to-file $kak_response_fifo -quoting shell -- %opt{some_list}" > $kak_command_fifo
eval "set -- $(cat $kak_response_fifo)"
}
```
If a %sh{} script refers to any variables multiple times they are all multiply
included in the environment. Example: if a %sh{} invocation refers to
${kak_buffile} 5 times, the environment will have "kak_buffile=..." repeated 5
times and so on. This repetition happens for each multiply used variable that
is passed into the environment.
The variable should, of course, be only passed into the environment once. This
commit should fix this issue.
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
This fixes an issue where completion would still be provided after
the closing character of a token, which could then get frustrating
combined with auto-insertion of completions.
For example, inserting `%{<newline>}` for a command-completed token
(such as the commands for a hook) would still trigger completion right
after the `}` and that completion would get auto-inserted **replacing**
that closing `}`.
Do not use a shared kakoune/ directory for all users to avoid the
complexity of having to set the sticky bit on that dir, resolve the
session directoy only once by using a static variable and an
immediately evaluated lambda.
This fixes an annoyance whenver using `su` and having Kakoune refuse
to start due to XDG_RUNTIME_DIR still being set.
For historical reasons, mouse events represent keyboard modifiers as a bitfield,
but keyboard events represent modifiers as a bitfield-plus-one. For example, a
mouse event with an Alt modifier will use the value 4, but a keyboard event will
use the value 5.
Previously, I refactored the parse_mask() helper to do the subtraction itself,
instead of requiring the caller to do it. This made keyboard-event decoding much
cleaner, but I didn't realise it broke mouse-event decoding. Now the subtraction
is done only for keyboard events.
Fixes#4176.
