Change the initial <c-h>/<c-k> bindings to the recently freed-up
<a-u></a-U>.
Pros:
- easier to remember
- the redo binding is logical.
- works on legacy terminals, unlike <c-h>
Cons:
- It's less convenient to toggle between selection undo and redo
keys. I think this is okay since this scenario does not happen that
often in practice.
Whenever a new history node is committed after some undo steps, instead
of creating a new branch in the undo graph, we first append the inverse
modifications starting from the end of the undo list up to the current
position before adding the new node.
For example let's assume that the undo history is A-B-C, that a single undo
has been done (bringing us to state B) and that a new change D is committed.
Instead of creating a new branch starting at B, we add the inverse of C
(noted ^C) at the end, and D afterwards. This results in the undo history
A-B-C-^C-D. Since C-^C collapses to a null change, this is equivalent to
A-B-D but without having lost the C branch of the history.
If a new change is committed while no undo has been done, the new history
node is simply appended to the list, as was the case previously.
This results in a simplification of the user interaction, as two bindings
are now sufficient to walk the entire undo history, as opposed to needing
extra bindings to switch branches whenever they occur.
The <a-u> and <a-U> bindings are now free.
It also simplifies the implementation, as the graph traversal and
branching code are not needed anymore. The parent and child of a node are
now respectively the previous and the next elements in the list, so there
is no need to store their ID as part of the node.
Only the committing of an undo group is slightly more complex, as inverse
history nodes need to be added depending on the current position in the
undo list.
The following article was the initial motivation for this change:
https://github.com/zaboople/klonk/blob/master/TheGURQ.md
The previous code was assuming it was fine to push_next without
growing, which used to be the case with the previous implementation
because we always have poped the current thread that we try to push.
However now that we use a ring-buffer, m_next_begin == m_next_end can
either mean full, or empty. We solve this by assuming it means empty
and never allowing the buffer to become full, which means we need
to grow after pushing to next if we get full.
Fixes#4859
The macOS CI manges to trigger this race. When it happens the
"c" inserted by the last command is not seen by the test runner.
Let's fix this by adding yet another sleep.
Recent changes for selection-undo added an assertion that triggers
when a mouse-drag overlaps with an insert mode, because both events
record selection history. However this is actually fine. The one
that finishes last concludes the selection edition, while the other
one will be a nop.
The test could be simpler (i.e. not require sleeps) but I figured it
doesn't hurt add this since we don't have any comparable tests.
After buffer modification - in particular after deletion - adjacent
selection history entries may correspond to the same effective
selection when applied to the current buffer. This means that we
sometimes need to press <c-h> multiple times to make one visible
change. This is not what the user expects, so let's keep walking the
selection history until we hit an actual change.
Alternatively, we could minimize the selection history after buffer
changes but I think that would make the it worse after content
undo+redo.
Each selection undo operation is surrounded by pair of
begin_edition()/end_edition() calls.
The original reason for adding these was that in one of my preliminary
versions, a WinDisplay hook could break an undo chain, even if the
hook did not affect selections at all. This has since been fixed.
By surrounding the undo with begin_edition()/end_edition(), try to
ensure that any selection modification that happens in a WinDisplay
hook would not break the undo chain. Essentially this means that,
after using <c-h> to undo a buffer change, this was meant to
make sure that <c-k> could redo that buffer change.
However, it turns out this actually doesn't work. The attached test
case triggers an assertion. As described in the first paragraph,
the only real-world motivation for this is gone, so let's simplify
the behavior.
The assertion fix means that we can test the next commit better.
With overlapping selections, pasting after breaks assumption of
SelectionList::for_each as our changes are no longer happening in
increasing locations.
We hence cannot rely on the ForwardChangeTracker in that case and
have to rely on the more general (and more costly) ranges update logic.
This interacts poorly with paste linewise pastes and we try to preserve
the current behaviour by tracking the last paste position.
Overall, this change really begs for overlapping selections to be
removed, but we will fix them like that for now.
Fixes#4779
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
Make the column highlighter faces final, and change final logic to
give precedence to the base face when both the base and new face are
final.
Fixes#4669
Always start with full buffer lines and trim the display buffer at
the very end, treat non-range display atoms as non-trimable in that
case and keep track of how many columns are occupied by "widgets"
such as line numbers or flags.
Fixes#4659
`x` is often criticized as hard to predict due to its slightly complex
behaviour of selecting next line if the current one is fully selected.
Change `x` to use the previous `<a-x>` behaviour, and change `<a-x>` to
trim to fully selected lines as `<a-X>` did.
Adapt existing indentation script to the new behaviour
The canonical way to disable all auto-insertion hooks is
set-option global disabled_hooks .*-insert
A recent change allowed to disable hooks that insert ) and }
independent of hooks that insert // (a step in the right
direction, we should do it for more filetypes).
Since the new hook ("go-insert-closing-delimiter") doesn't match
.*-insert, it broke the above snippet. Fix this by renaming it to
"go-closing-delimiter-insert".
This makes it a bit less obvious how to disable only comment insertion.
Not sure if there's interest in that, but make it easier by renaming
"go-insert" to "go-comment-insert".
Changed the indentation behavior such that an extra level of
indentation is added after a line containing a ( or { that is
not closed on the same line instead of aligning to the unclosed
( or {. This is consistent with how `go fmt` formats source code.
Added regression tests.
When indenting on newline in Go files, only remove trailing whitespace
on the previous line and copy indentation of the previous line if in
comment context.
Added regression tests.
Insert mode completions are accepted by typing any key. For example,
if there is a completion "somefunction()", then typing
some<c-n>;
will insert
somefunction();
and then the InsertCompletionHide hook will fire. The hook parameter
is a range that contains the entire thing: the actual completion plus
the trailing semicolon that closed the completion menu.
The [original motivation] for the hook parameter was to support
removing text inserted by completion, so we can apply text edits
or expand snippets instead. One problem is that we don't want to
remove the semicolon. Another problem came up in a discussion
about [snippets]: let's say we have a snippet "add" that expands to
add(?, ?)
where ? are placeholders. After snippet expansion the cursor replaces
the first placeholder. If I type "ad<c-n>1" I expect to get "add(1, ?)".
If the InsertCompletionHide hook only runs after processing the "1"
keystroke, this is not possible without evil hacks.
Fix these problems by running InsertCompletionHide when a completion is
accepted _before_ inserting anything else into the buffer. This should
make it much easier to fully implement [LSP text edits]. I doubt
that anyone besides kak-lsp is using the hook parameter today so this
should be a low-risk fix.
[original motivation]: https://github.com/mawww/kakoune/issues/2898
[snippets]: https://github.com/kak-lsp/kak-lsp/pull/616#discussion_r883208858
[LSP text edits]: https://github.com/kak-lsp/kak-lsp/issues/40
Some languages have a trim-indent command but don't use it (for no
apparent reason). Make them trim trailing spaces when exiting insert
mode, like most other languages support scripts do.
This approach is not very elegant as it hooks into the event manager
deep inside the call graph, but solves the exiting issue and is an
okay stop gap solution until a better design comes up.
Fixes#4605
Deleting a buffer resets normal mode on all clients that were
displaing that buffer, but ScopedForceNormalMode that are used
from user mode do not take this possiblity into account on
destruction, which leads to deleting the last normal mode from
the context, ending up with an empty mode stack.
Fixes#3909
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
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
