If I use "man dirname" and select "basename"
SEE ALSO
basename(1), readlink(1)
^------^
then pressing <ret> to trigger man-jump selects everything from "ALSO"
until "basename(1)" Obviously that's not the name of a man page,
so it fails. When I select only "asename" it works.
The bad selection happens because we use a combination of <a-?> and ?
to extend the selection to a full link. This is more complicated than
it needs to be; let's just select the surrounding WORD. This works
fine because man page links never start mid-word, and trailing
characters are ignored anyway.
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.
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.
I dedicate any and all copyright interest in this software to the
public domain. I make this dedication for the benefit of the public at
large. I intend this dedication to be an overt act of relinquishment in
perpetuity of all present and future rights to this software under
copyright law.
Instead of triming only buffer ranges, add a trim_from method to
display line to keep the initial N columns, we know how many columns
are used by non-trimable widgets in DisplaySetup::widget_columns so
we can just pass this.
Also restore the previous logic for face merging
Fixes#4670
