Commit 69053d962 (Use menu behavior when completing change-directory,
2022-07-19) made ":cd dir/" actually run ":cd dir/first-subdir",
which can be surprising.
This is usually irrelevant because you rarely type the trailing slash.
However it does happen after correcting an error with `<backspace>`
and friends. For for example,
:cd d<tab>/f<backspace>
results in
:cd dir/
We should probably fix user expectations here. Do this by adding "dir/"
as valid completion. This requires us to allow empty candidates in
"RankedMatch" but there's no harm in that. This means we need to
filter out empty completions from shell-script-candidates elsewhere.
Alternative fix: we could revert 69053d962. This would remove the
convenient menu behavior but that wouldn't be a huge deal.
Fixes#4775
Mapping upper case keys is legitimate, for exampled so that they behave
the same as a lower case mapping. The current rejection of those mappings
is a misguided attempt to prevent mapping *to* upper case keys as those
will never get triggered.
Closes#4769
After a failed
:write file-that-already-exists
a user might want to type ":<up> -f<ret>" to force-overwrite.
This doesn't work because :write's switches must precede the filename.
It's dual :edit does not have this restriction.
Some commands require switches to precede positional arguments for a
good reason; for example because positional arguments might start with
"-" (like ":echo 1 - 1").
There seems to be no reason for the :write restriction, so remove
it. Same for :enter-user-mode.
Thanks to alexherbo2 for reporting.
To be able to undo selection changes, we want to record selections
from all commands that modify selections. Each such command will get
its own private copy of the selections object.
This copy will live until the command is finished executing.
All child commands that are run while the command is executing,
will also use the same copy, because to the user it's all just one
selection change anyway.
Add an RAII object in all places where we might modify selections.
The next commit will use this to create the private selections copy
in the constructor (if there is none) and remove redundant history
items in the destructor.
We could avoid the RAII object in some places but that seems worse.
For lifetimes that don't correspond to a lexical scope, we use a
std::unique_ptr. For lambdas that require conversion to std::function,
we use std::shared_ptr because we need something that's copyable.
When passing a filename parameter to "write", the -force parameter
allows overwriting an existing file.
The "write!" variant (which allows writing files where the current
user does not have write permissions) already implies -force.
All other variants (like write-quit or write-all) do not take a
file parameter.
Hence -force is relevant only for "write". Let's hide it from the
autoinfo of the other commands.
It's difficult to avoid duplication when constructing the constexpr
SwitchMap because String is not constexpr-enabled. Today, all our
SwitchMap objects are known at compile time, so we could make SwitchMap
use StringView to work around this. In future we might want to allow
adding switches at runtime, which would need String again to avoid
lifetime issues.
When I wrote this line I wanted to avoid adding the array size but
I didn't know about make_array().
I had unsuccessfully tried some alternatives, for example
Array{"a", "b", "c"}
which doesn't work because we need StringView (c.f. git blame on
this line)
also
Array<StringView>{"a", "b", "c"}
doesn't work because it's missing a template argument.
This makes the function easier to find for newcomers because
to_string() is the obvious name. It enables format() to do the
conversion automatically which seems like good idea (since there is
no other obvious representation).
Of course this change makes it a bit harder to grep but that's not
a problem with clang tooling.
We need to cast the function in one place when calling transform()
but that's acceptable.
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 allows to select completions without pressing Tab.
There are two different obvious ways to add the menu bit.
1. When creating the "Completions" object, pass the
Completions::Flags::Menu parameter.
2. If there is a completer function like "complete_scope", wrap
it, e.g. "menu(complete_scope)".
I have settled on always using 2 if there is a completer function
and 1 otherwise.
The advantage of 2 over 1 is that it allows to use the completer
function in a context where we don't want the menu behavior
(e.g. "complete-command").
---
Now the only* completion type where we usually don't use menu behavior
is file completion. Unfortunately, menu behavior has poor interaction
with directories' trailing slashes. Consider this (contrived) example:
define-command ls -docstring "list directory contents" -params .. %{
echo -- %sh{ls "$@"}
}
complete-command -menu ls file
Run ":ls kakoun<ret>". The prompt expands to ":ls kakoune/"
before executing. Next, run ":<c-p>". This recalls ":ls kakoune/"
and immediately selects the first completion, so on validation,
the command will be ":ls kakoune/colors/", which is weird.
[*] Also, expansions like %val{bufname} also don't use menu
behavior. It wouldn't add value since validation doesn't add a
closing delimiter. I have an experimental patch that adds closing
delimiters automatically but I'm not sure if that's the right
direction.
This makes "cd<space><ret>" change to the first completion,
not to $HOME. This might be surprising but it should make sense.
I don't have a concrete motivation but this should save a Tab press
in some scenarios.
We allow to abbreviate scopes ("set g" means the same thing as "set
global") but that feature is a bit obscure. Users might figure out the
menu completion behavior faster, so let's maybe use it here as well?
Not really attached to this but it enables the next commit to use
menu() for completing scopes.
This refactoring is possible because we always have
params[token_to_complete].length() == pos_in_token
---
Instead of three separate functions, I originally tried to add
template arguments to complete_scope(). That worked fine with g++
12.1 but clang 14.0 complained when wrapping a menu() around a
complete_scope() that relied on defaulted template arguments:
commands.cc:1087:20: error: no matching function for call to 'menu'
make_completer(menu(complete_scope), menu(complete_hooks), complete_nothing,
^~~~
commands.cc:116:6: note: candidate template ignored: couldn't infer template argument 'Completer'
auto menu(Completer completer)
^
On a command prompt like
"set-option -remove buffer aligntab "
we fail to show the aligntab-specific info . Fix this by skipping a
leading -remove, just like we skip -add.
Add an explicit specialization of contains() because otherwise I'd
need to write something like
contains(Array{"-add", "remove"}, param)
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.
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.
"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.
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.
