-
Notifications
You must be signed in to change notification settings - Fork 719
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add -quote argument to kakoune #5211
base: master
Are you sure you want to change the base?
Conversation
One reason for using |
At least for me, $ du -Dh (which kak)
53M ...
$ du -Dh (which sed)
140K ...
$ du -DhL (which sed-static)
276K ...
$ time sh -c 'kakquote() { printf "%s" "$*" | sed "s/'\''/'\'''\''/g; 1s/^/'\''/; \$s/\$/'\''/"; }
kakquote github
kakquote kakoune
kakquote quoting'
'github''kakoune''quoting'
________________________________________________________
Executed in 23.63 millis fish external
usr time 15.54 millis 0.00 micros 15.54 millis
sys time 9.43 millis 774.00 micros 8.65 millis
$ time sh -c '
kak -quote kakoune -- github
kak -quote kakoune -- kakoune
kak -quote kakoune -- quoting'
'github''kakoune''quoting'
________________________________________________________
Executed in 16.39 millis fish external
usr time 9.36 millis 0.00 micros 9.36 millis
sys time 6.99 millis 705.00 micros 6.29 millis
$ time sh -c 'kakquote() { printf "%s" "$*" | sed-static "s/'\''/'\'''\''/g; 1s/^/'\''/; \$s/\$/'\''/"; }
kakquote github
kakquote kakoune
kakquote quoting'
'github''kakoune''quoting'
________________________________________________________
Executed in 9.42 millis fish external
usr time 3.54 millis 346.00 micros 3.19 millis
sys time 7.10 millis 122.00 micros 6.98 millis I don't think that EDIT: Add |
Not sure if this is worth it. When there are multiple layers of escaping, things can indeed get complicated but those cases are infrequent. For the occasional complex task it's best to use a different scripting language instead of shell. |
BTW it's interesting that your (presumably |
Agree it's fun that the kak version is faster that sed - possibly something to do with initialising and parsing the sed script? mmap()ing the kak binary is going to be pretty fast as it's already mapped for the running editor that called the script, although ld.so still has to resolve everything each time. One place kakoune-quoting from shell code is invariably needed is when using printf "echo -to-file '%s' %%val{selection}\n" \
"${kak_response_fifo//\'/\'\'}" > "$kak_command_fifo"
dosomething < "$kak_response_fifo" but POSIX has failed to standardise a lot of these expansions, leading to expensive fork+exec out for trivial tasks if you can't assume a sufficiently capable shell. (I'd love the Austin Group to be less indolent; alas we are where we are.) As far as I can see, many kak scripts just get this wrong and do printf 'echo -to-file %s %%val{selection}\n' \
"$kak_response_fifo" > "$kak_command_fifo" or worse, interpolate I note that quote() {
while [ "${1#*\'}" != "$1" ]; do
printf "%s''" "${1%%\'*}"
set -- "${1#*\'}"
done
printf '%s\n' "$1"
} This never execs anything and only loops once for each printf "echo -to-file '%s' %%val{selection}\n" \
"$(quote "$kak_response_fifo")" > "$kak_command_fifo"
dosomething < "$kak_response_fifo" doesn't obfuscate too badly either. Edit: use quote() {
printf "'"
while [ "${1#*\'}" != "$1" ]; do
printf "%s''" "${1%%\'*}"
set -- "${1#*\'}"
done
printf "%s'\n" "$1"
} if you prefer your quoted version to include the outer single-quotes as with the sed-script above. The Edit 2: For me, your sed-based |
I spent some time investigating writing a quoting function in shell in #3340 (comment) - the fastest implementation I came up with used |
Ah, interesting. For me, your case version is around 67us for the "Don't Let's Start" test string you benchmark, my quote function above is 59us, the sed script takes 1.26ms, and just running /bin/true is around 0.98ms, bounding below the time a lighter-weight sed could manage. So on my system, your and my shell functions are 19-21x faster. I'm guess the key difference between my test and yours is that you used dash, which I can believe is a lot slower than bash. It would be fairly easy to squeeze out more performance, e.g.
should be a bit quicker (about 10% for me), but I'd imagine these will be diminishing gains vs massive win of not exec()ing out at all unless there's something very wrong with the one of the builtins on whichever shell is under test.
(Though to be honest, I'll do none of these things locally. I just define /bin/sh as a decent shell on my systems and use |
@arachsys In your example if FIFOs were available as echo 'echo -to-file %val{response_fifo} -- %val{selection}' > "$kak_command_fifo" Not sure it’s possible though. EDIT: You can also use the |
I'm not sure how tricky this would be either, but it would certainly encourage people not to make the mistake of badly-quoting this common idiom. 👍
This is the wrong kind of quoting, although I'm sure people have mistakenly used it in kak scripts. It does shell quoting, which replaces One could definitely imagine an alternative (Perhaps one good thing to do here is to put a clean, well-written shell function to kak-quote strings in the documentation, so people don't write something worse or fail to realise it's necessary. If you're sticking to strict POSIX, remember that POSIX doesn't even manage to specify |
Congratulations, @arachsys, you have the fastest Kakoune-quoting implementation in pure POSIX shell. :) I added this to the benchmark harness I linked to previously, and that implementation won handily over all the others I'd tried. I've now made a thread on the forum so these tricks won't be forgotten in some random PR comment.
It turns out, this is not the case! Running in my benchmark harness, your
Oh my, yes. This stuff would definitely be a rounding error in most Kakoune plugins. |
Many kakoune scripts evaluate kak script printed from
%sh
blocks.Some of these script want to interpolate strings into the printed script and need to escape these strings properly.
Common cases that need to be escaped are interpolated kakoune and shell commands.
This is currently done using
sed
scripts which are duplicated across many files.Kakoune already provides all the necessary quoting logic in e.g. the
echo
command with its-quoting
switch.This PR adds a
-quote
switch which allows scripts to usekak -quote kakoune
orkak -quote shell
instead ofsed
and applies these changes to some.kak
scripts.I personally think that the quoting in e.g.
menu.kak
becomes way more readable this way.