Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

PJungkamp
Copy link
Contributor

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 use kak -quote kakoune or kak -quote shell instead of sed 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.

@Screwtapello
Copy link
Contributor

This is currently done using sed scripts which are duplicated across many files.

One reason for using sed is that it's pretty quick to start up. On my machine, the GNU sed binary is 124K, while the Kakoune binary is 57M - I worry that this approach is going to slow things down.

@PJungkamp
Copy link
Contributor Author

PJungkamp commented Aug 5, 2024

At least for me, kak -quote kakoune is actually faster than the kakquote sh function using sed for simple cases. sed can be faster if I use a statically linked version.

$ 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 sed-static is representative of sed performance since most distros seem to ship a dynamically linked sed.

EDIT: Add du -Dh for executable size.
EDIT: Add sed-static test.

@krobelus
Copy link
Contributor

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.
For menu.kak specifically, I've been using a version that renders the menu into a scratch buffer (see my menu branch).
The UI is not without surprises but its usable to me

@krobelus
Copy link
Contributor

BTW it's interesting that your (presumably -O0) kak.debug is faster than the sed solution. A kak.opt binary is much smaller

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants