When running modeline-parse on this file:
# kakoune: filetype=ledger:indentwidth=4
I get this error from dash (and a similar one from bash):
sh: 53: readonly: key: is read only
This is because the readonly variable "key" is used elsewhere, both
times as global. Fix this by making both variables local. While
at it, remove an unused variable.
Fixes#4478
This is equivalent to a change to grep.kak in 649e252f7 (bring *grep*
buffer to front in context of toolsclient, 2020-08-14).
If a toolsclient is set, make-next-error (and make-previous-error) will
jump to %opt{make_current_error_line}. This is wrong if the toolsclient
does not show the *make* buffer. In that case make_current_error_line
is undefined and we end up showing the goto menu. This has occasionally
been annoying me for a long time but I never bothered investigating.
Fix this by switching to the *make* buffer. The potential downside
is if make-next-error is run from the toolsclient, where we no longer
jump to the error but that's fine because we can use <ret>.
We can maybe improve this later by extending the logic, see
https://github.com/mawww/kakoune/pull/3656#pullrequestreview-472052285
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.
I can't think of a case where a URL would not start at a word boundary.
Let's add that to the regex. In addition to correctness, this also
slightly improves performance because matching can stop earlier.
$ HOME=$PWD hyperfine -w 1 'git checkout HEAD'{~,}' -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy'
Benchmark 1: git checkout HEAD~ -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy
Time (mean ± σ): 1.123 s ± 0.022 s [User: 1.100 s, System: 0.027 s]
Range (min … max): 1.093 s … 1.174 s 10 runs
Benchmark 2: git checkout HEAD -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy
Time (mean ± σ): 1.019 s ± 0.026 s [User: 1.001 s, System: 0.021 s]
Range (min … max): 0.984 s … 1.051 s 10 runs
Summary
'git checkout HEAD -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy' ran
1.10 ± 0.04 times faster than 'git checkout HEAD~ -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy'
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 and to the detriment of my heirs and successors. 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.
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 and to the detriment of my heirs and successors. 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.
Running %sYeti<ret>casdf on file
[example.journal.txt](https://github.com/mawww/kakoune/issues/4685#issuecomment-1193243588)
can cause noticeable lag. This is because we insert text at 6000
selections, which means we need to update highlighters in those lines.
The runtime for updating range highlighters is quadratic in the
number of selections: for each selection, we call on_new_range(),
which calls add_matches(), which calls std::rotate(), which needs
needs linear time.
Fix the quadratic runtime by calling std::inplace_merge() once instead
of repeatedly calling std::rotate(). This is works because ranges
are already sorted.
I used this script to benchmark the improvements.
(In hindsight I could have just used "-ui json" instead of tmux).
#!/bin/sh
set -ex
N=${1:-100}
kak=${2:-./kak.opt}
for i in $(seq "$N")
do
echo -n "\
2022-02-06 * Earth
expense:electronics:audio 116.7 USD
liability:card -116.7 USD
2022-02-06 * Blue Yeti USB Microphone
expense:electronics:audio 116.7 USD
liability:card -116.7 USD
"
done > big-journal.ledger
echo > .empty-tmux.conf 'set -sg escape-time 5'
test_tmux() {
tmux -S .tmux-socket -f .empty-tmux.conf "$@"
}
test_tmux new-session -d "$kak" big-journal.ledger
test_tmux send-keys '%sYeti' Enter c 1234567890
sleep .2
test_tmux send-keys Escape
while ! test_tmux capture-pane -p | grep 123
do
sleep .1
done
test_tmux send-keys ':wq' Enter
while test_tmux ls
do
sleep .1
done
rm -f .tmux-socket .empty-tmux.conf
This script's runtime used to grow super-linearly but now it grows
linearly:
kak.old kak.new
N=10000 1.142 0.897
N=20000 2.879 1.400
Detailed results:
$ hyperfine -w 1 './bench.sh 10000 ./kak.opt.'{old,new}
Benchmark 1: ./bench.sh 10000 ./kak.opt.old
Time (mean ± σ): 1.142 s ± 0.072 s [User: 0.252 s, System: 0.059 s]
Range (min … max): 1.060 s … 1.242 s 10 runs
Benchmark 2: ./bench.sh 10000 ./kak.opt.new
Time (mean ± σ): 897.2 ms ± 19.3 ms [User: 241.6 ms, System: 57.4 ms]
Range (min … max): 853.9 ms … 923.6 ms 10 runs
Summary
'./bench.sh 10000 ./kak.opt.new' ran
1.27 ± 0.09 times faster than './bench.sh 10000 ./kak.opt.old'
$ hyperfine -w 1 './bench.sh 20000 ./kak.opt.'{old,new}
Benchmark 1: ./bench.sh 20000 ./kak.opt.old
Time (mean ± σ): 2.879 s ± 0.065 s [User: 0.553 s, System: 0.126 s]
Range (min … max): 2.768 s … 2.963 s 10 runs
Benchmark 2: ./bench.sh 20000 ./kak.opt.new
Time (mean ± σ): 1.400 s ± 0.018 s [User: 0.428 s, System: 0.083 s]
Range (min … max): 1.374 s … 1.429 s 10 runs
Summary
'./bench.sh 20000 ./kak.opt.new' ran
2.06 ± 0.05 times faster than '../repro.sh 20000 ./kak.opt.old'
LineRangeSet::add_range() calls Vector::erase() in a loop over the
same vector. This could cause performance problems when there are many
selections. Fix this by only calling Vector::erase() once. I didn't
measure anything because my benchmark is dominated by another issue
(see next commit).
LineRangeSet::remove_range() also has a suspicious call to erase()
but that one is only used in test code, so it doesn't matter.
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 and to the detriment of my heirs and successors. 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.
There have been proposals to add more language aliases to markdown.kak
(#4592) and allow users to add their own aliases (#4489).
To recap: various markdown implementations allow specifying aliases
for languages. For example, here is a code block that should be
highlighted as filetype "haskell" but isn't:
```hs
-- highlight as haskell
```
There are lots of aliases out in the wild - "pygmentize -L" lists
some but I don't think there is a canonical list.
Today we have a hardcoded list of supported filetypes. This is hard
to mainta, extend, and it can impact performance.
This patch simply attempts to load the module "hs" and the shared
highlighter "hs". This means that users can use this (obvious?) snippet
to add their own aliases:
provide-module hs %{
require-module haskell
add-highlighter shared/hs ref haskell
}
Untrusted Markdown files can load arbitrary modules, but that was
already true before, and modules are assumed to be trusted anyway.
Since language highlighters are now loaded *after* the generic
code-block highlighter, we need to make sure the language highlighters
take precedence. Do this by making them sub-regions of the generic one.
Closes#4489
This improves performance on the [5MB Markdown
file](https://github.com/mawww/kakoune/issues/4685#issuecomment-1208129806).
$ HOME=$PWD hyperfine -w 1 'git checkout HEAD'{~,}' -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy'
Benchmark 1: git checkout HEAD~ -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy
Time (mean ± σ): 3.225 s ± 0.074 s [User: 3.199 s, System: 0.027 s]
Range (min … max): 3.099 s … 3.362 s 10 runs
Benchmark 2: git checkout HEAD -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy
Time (mean ± σ): 1.181 s ± 0.030 s [User: 1.162 s, System: 0.021 s]
Range (min … max): 1.149 s … 1.234 s 10 runs
Summary
'git checkout HEAD -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy' ran
2.73 ± 0.09 times faster than 'git checkout HEAD~ -- :/rc/filetype/markdown.kak && ./kak.opt big_markdown.md -e "hook global NormalIdle .* quit" -ui dummy'
(These numbers depend on another optimization.)
Filetypes markdown and restructuredtext reuse highlighters from other
filetypes to highlight code blocks. For example, to highlight a code
block of language foo they essentially do
require-module foo
add-highlighter [...] ref foo
This works great if the module name matches the shared
highlighter. This is the case almost all scripts in rc/filetype*.
The only exception is kakrc.kak: the highlighter is named "kakrc"
(just like the filetype) but the module is named "kak".
This requires weird hacks in markdown/restructuredtext. Ideally we
could remove this inconsistency by renaming both the filetype and the
highlighter to "kak" but that's a breaking change. Until we do that,
let's add an alias so we can treat filetypes uniformly. This helps
the following commits, which otherwise would need to add ugly extra
code for kakrc highlighters.
The following commit will generalize this approach, allowing users
to add arbitrary aliases.
From the issue:
> It often happens to me that I carefully craft a selection with multiple
> cursors, ready to make changes elegantly, only to completely mess it
> up by pressing a wrong key (by merging the cursors for example). Being
> able to undo the last selection change (even if only until the previous
> buffer change) would make this much less painful.
Fix this by recording selection changes and allowing simple linear
undo/redo of selection changes.
The preliminary key bindings are <c-h> and <c-k>.
Here are some other vacant normal mode keys I considered
X Y
<backspace> <minus>
# ^ =
<plus> '
unfortunately none of them is super convenient to type. Maybe we
can kick out some other normal mode command?
---
This feature has some overlap with the jump list (<c-o>/<c-i>) and
with undo (u) but each of the three features have their moment.
Currently there's no special integration with either peer feature;
the three histories are completely independent. In future we might
want to synchronize them so we can implement Sublime Text's "Soft
undo" feature.
Note that it is possible to restore selections that predate a buffer
modification. Depending on the buffer modification, the selections
might look different of course. (When trying to apply an old buffer's
selection to the new buffer, Kakoune computes a diff of the buffers
and updates the selection accordingly. This works quite well for
many practical examples.)
This makes us record the full history of all selections for each
client. This seems wasteful, we could set a limit. I don't expect
excessive memory usage in practice (we also keep the full history of
buffer changes) but I could be wrong.
Closes#898
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.
The next commit changes the selections to a history of
selections. Today we directly access the selections data member. Let's
instead use an accessor method, to reduce the number of changes in
the next commit.
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.
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.
