Skip to content

format.kak: Make sure Kakoune provides the envvars formatcmd needs - #5520

Open
Screwtapello wants to merge 1 commit into
mawww:masterfrom
Screwtapello:pass-format-args
Open

format.kak: Make sure Kakoune provides the envvars formatcmd needs#5520
Screwtapello wants to merge 1 commit into
mawww:masterfrom
Screwtapello:pass-format-args

Conversation

@Screwtapello

Copy link
Copy Markdown
Contributor

When Kakoune executes a shell command, it scans the text for tokens like kak_* and if it finds any it recognises it adds them to the environment for the command (as described in :doc expansions shell-expansions). For example, if you select some text and type:

|fold -w $kak_opt_autowrap_column<ret>

...then Kakoune sees kak_opt_autowrap_column, and exports the value of the autowrap_column option into the environment of the shell, which expands the variable reference then runs the command.

The :format command amounts to something like:

|eval "$kak_opt_formatcmd"

...which means that Kakoune never sees "kak_opt_autowrap_column", so it never provides the value, so the shell winds up with a malformed command like "fold -w" which just causes an error.

To avoid this problem, as well as passing "$kak_opt_formatcmd" into the shell command, we also append it to the shell command where Kakoune can see it, guarded by an exit statement to prevent it from being processed.

When Kakoune executes a shell command, it scans the text for tokens like `kak_*`
and if it finds any it recognises it adds them to the environment for the
command (as described in `:doc expansions shell-expansions`). For example,
if you select some text and type:

    |fold -w $kak_opt_autowrap_column<ret>

...then Kakoune sees `kak_opt_autowrap_column`, and exports the value of the
`autowrap_column` option into the environment of the shell, which expands the
variable reference then runs the command.

The `:format` command amounts to something like:

    |eval "$kak_opt_formatcmd"

...which means that Kakoune never sees "kak_opt_autowrap_column", so it never
provides the value, so the shell winds up with a malformed command like
"fold -w" which just causes an error.

To avoid this problem, as well as passing "$kak_opt_formatcmd" into the shell
command, we also *append* it to the shell command where Kakoune can see it,
guarded by an `exit` statement to prevent it from being processed.
Comment thread rc/tools/format.kak
format_out="$(mktemp "${TMPDIR:-/tmp}"/kak-formatter.XXXXXX)"

cat > "$format_in"
eval "$kak_opt_formatcmd" < "$format_in" > "$format_out"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if there an obvious reason, but cannot we expand formatcmd directly here ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shell has special rules for expanding inside double quotes, so things work as expected even if the thing you're expanding includes double quotes:

$ q='"'
$ printf '%s\n' "a" "b" # actual double quotes delimit args
a
b
$ printf '%s\n' "a${q} ${q}b" # quotes resulting from expansion don't
a" "b

If Kakoune did the expansion instead of the shell, it wouldn't get the protection of the shell's "expand inside double quotes" logic. In particular, if formatcmd included a double quote and then a newline, the eval would be executed up until the newline, then whatever follows the newline would be executed as a separate command with the redirections.

As an alternative, it might be possible to re-implement the format plugin on top of Kakoune's | command, which would solve this problem, but would make it impossible to collect errors reported by the formatter as the current version of the plugin does. Perhaps there's a better way to get the best of both worlds, but I haven't thought of it yet.

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.

2 participants