clang/clangd complain about the new HashSet type:
hash_map.cc:98:20: warning: braces around scalar initializer [-Wbraced-scalar-init]
set.insert({10});
^~~~
The argument to HashSet<int>::insert is just an int, so we don't
need braces. Only an actual HashMap would need braces to construct
a HashItem object.
Instead of storing regexes in each regions, move them to the core
highlighter in a hash map so that shared regexes between different
regions are only applied once per update instead of once per region
Also change iteration logic to apply all regex together to each
changed lines to improve memory locality on big buffers.
For the big_markdown.md file described in #4685 this reduces
initial display time from 3.55s to 2.41s on my machine.
Recently, switch completion were given the menu behavior.
Unfortunately this breaks cases like
:echo -- -mark<ret>
where the hypothetical user wanted to actually display "-mark", not
"-markup".
Simply bail if there is a double-dash. This is not fully correct,
for example it wrongly disables switch completion on
echo -to-file -- -
but that's minor, we can fix it later.
In future, we should reuse the ParametersParser when computing completions,
which will obsolete this workaround.
So far they have only been talked about in source code
(HistoryRegister), not in documentation.
Let's give them an official name so users can find them better;
"prompt history register" seems better than "history register" since
the former gives more context. OTOH, in future other registers (like @)
could grow into history registers, so I'm not sure.
State that the history registers are only changed by _interactive_
prompts; that's not quite true yet for user modes but I'll push a
separate fix for that.
We often use the pattern «map global normal ": foo"». The space
after the colon is unnecessary since execution of the mapping won't
add to history anyway, since 217dd6a1d (Disable history when executing
maps, 2015-11-10).
With the parent commit, the space is no longer necessary for user
mappings, so there is no reason to continue the cargo-cult.
Remove the space from mappings to set a good example.
Commit 217dd6a1d (Disable history when executing maps, 2015-11-10)
made it so with
map global normal X %{:echo 123<ret>}
X does not add to prompt history (%reg{:}).
Unfortunately this behavior was not extended to mappings in the "user"
keymap, nor to mappings in custom user modes.
In my experience, not adding to history is almost always the expected
behavior for mappings. Most users achieve this by adding a leading space:
map global user X %{: echo 123<ret>}
but that's awkward. We should have good defaults (no nnoremap)
and map should work the same way across all modes.
Fix this by also disabling history when executing user mappings. This
is a breaking change but I think it only breaks hypothetical scenarios.
I found some uses where user mappings add to history but none of them
looks intentional.
f702a641d1/.config/kak/kakrc (L169)604ef1c1c2/kakrc (L96)d22e7d6f68/kak/kakrc (L71)https://grep.app/search?q=map%20%28global%7Cbuffer%7Cwindow%29%20user%20.%2A%5B%21%3A/%5D%5B%5E%20%5D.%2A%3Cret%3E®exp=true
This gives the "prompt" command the same "-menu" switch as
"complete-command" and "define-command". I don't think anyone has
asked for it but it seems like a good idea?
Both "define-command" and "prompt" use the same logic, so share
it. This will make it easy to implement "prompt -menu".
This reveals a problem with PromptCompleterAdapter: when converting
it to std::function and then to bool, it always evaluates to true
because it has an operator(). However, it should evaluate to false
if the adaptee holds no valid function (e.g. is a default-constructed
std::function). Otherwise we try to call a non-existant function. Tweak
PromptCompleterAdapter to work for empty inputs.
If I type
:echo -mx
I get no completions, even when I move the cursor on the x.
If I replace the x with a k, I get a completion "-markup".
The odd thing is that if I accept the completion while the cursor is
on the k, then the commandline will be
:echo markupk
Evidently, the characters under cursor (x/k) influence the completion
(actually all letters right of the cursor do), but they are not
incorporated in the final result, which is weird.
I think there are two consistent behaviors:
1. Compute completions only from characters left of the cursor. We already
do this in insert mode completion, and when completing the command name
in a prompt.
2. Compute completions from the whole token, and when accepting a completion,
replace the whole token.
Most readline-style completion systems out there implement 1. A
notable exception is fish's tab-completion. I think we should stick
to 1 because it's more predictable and familiar. Do that.
This allows us to get rid of a special case for completing command
names, because the new behavior subsumes it.
In fact, I think this would allow us to get rid of most "pos_in_token"
or "cursor_pos" completer parameters. I believe the only place where we
it's actually different from the end of the query is in "shell-script"
completers, where we expose "$kak_pos_in_token". I think we can still
remove that parameter and just cut the commandline at the cursor
position before passing it to a "shell-script" completer. Then we
also don't need "$kak_token_to_complete" (but we can still keep
expose them for backwards compatibility).
Just like in the parent commit, this requires us to use a non-owning
type. Technically, these strings all benefit from SSO, so there is
no lifetime issue, but we can't deduce that from the types.
I guess we could use InplaceString just as well.
This means that typing
:add-highlighter g c 80
results in
:add-highlighter global/ column 80
Paths for add-highlighter do not get the menu behavior because we
want to be able to type "global/foo" even if "global/foobar" exists.
complete() merely iterates over its input, so we can pass it a range
instead of a vector. For some reason, this requires specifying the
type of the static array, or else its elements become String which
triggers this assertion:
static_assert(not std::is_same<decltype(*begin(container)), String>::value,
"complete require long lived strings, not temporaries");
Specify the type with an explicit Array<StringView, 8>. This is
pretty ugly but the alternative of appending "_sv" to every single
array element seems worse?
If we modify the vector of user modes while complete() is iterating
over, we could crash -- but that scenario is impossible since both
only happen inside the single-threaded server process.
We already use the menu behavior in complete_command_name(); let's do
the same for switches, since we can complete all valid inputs and
it can save a Tab key in some scenarios.
CommandManager::complete is fairly complex. We can't expect callers
to add the menu bit when appropriate, so we currently do it here.
The next commit will give switch completions the menu behavior, so this
is necessary so we can still type "echo --" without an auto-expansion
to "echo -to-file".
"complete_alias_name" is a better name then "complete_alias" because
it's consistent with more similar names, which are:
complete_client_name
complete_command_name
complete_module_name
complete_option_name
complete_register_name
complete_scope
complete_face
We define
using PromptCompleter = std::function<Completions (const Context&, CompletionFlags,
StringView, ByteCount)>;
Some command completers are *almost* convertible to PromptCompleter;
the only difference is the string type of the prefix argument.
The later commits in this series want to use menu() on the
completers. Enable this by harmonizing the types.
