kakoune icon indicating copy to clipboard operation
kakoune copied to clipboard

[BUG] Markup returned from expansions is not parsed in the modeline

Open raiguard opened this issue 2 years ago • 4 comments

Version of Kakoune

v2021.11.08

Reproducer

Execute the following commands:

declare-option str modeline_test "{green}testing"
set-option global modelinefmt '%opt{modeline_test}'

Outcome

The modeline is printed with markup strings included, and the markup strings are not parsed.

image

Expectations

The markup strings would be parsed. This would require a second pass on the modeline, but would be worth it in order to facilitate easier customization. As it is now, you have to hardcode the markup strings and duplicate logic in multiple places if you want things to be dynamically added/removed to/from the modeline.

Additional information

No response

raiguard avatar Jan 17 '22 08:01 raiguard

An even simpler way to reproduce:

kak -n -e "set global modelinefmt '%{{Error}test}'"

Note that the following variants all work as expected:

kak -n -e "set global modelinefmt '{Error}test'"
kak -n -e "set global modelinefmt '{Error}%{test}'"

The documentation for the modelinefmt option says:

that string is first expanded as a command line would be (expanding '%…​{…​}' strings), then markup tags are applied

...so it sounds like it should be possible for an expansion to produce markup tags.

Screwtapello avatar Jan 17 '22 08:01 Screwtapello

Searching the codebase for modelinefmt, we find the place it's used in Client::generate_mode_line():

https://github.com/mawww/kakoune/blob/9acd4e62dc485aa7e44a601a0300697f8825a98c/src/client.cc#L160-L161

When expansions are... expanded, there's a post-processing step that escapes each { in the resulting string with {, preventing it from being interpreted as a face. This escaping was added in 2016 in 03eb128536acb3d870b7010ae3cad3d2b707cf72 (clearly titled "Ensure content of expanded strings in modelinefmt is not interpreted as markup"), but there's no rationale in the commit message or link to an issue or discussion.

Screwtapello avatar Jan 17 '22 08:01 Screwtapello

Now that I think about it, the reason for automatically escaping expansions is probably that the default modelinefmt is:

%val{bufname} %val{cursor_line}:%val{cursor_char_column} {{context_info}} {{mode_info}} - %val{client}@[%val{session}]

cursor_line and cursor_char_column are safe because they're only numbers, client and session are safe because Kakoune enforces that they are ~alphanumeric, and context_info and mode_info are treated specially, but bufname can contain arbitrary text including things that look like markup.

To fix this, we'd need one of:

  • a way to distinguish "expansions that can include markup" from "expansions that cannot include markup"
  • change the default modelinefmt to replace %val{bufname} with %sh{echo "$kak_bufname" | sed -e 's/{/\\{/g'} (which would be slow at runtime)
  • put up with weird modeline glitches for very unusually-named files.

Screwtapello avatar Jan 18 '22 04:01 Screwtapello

Brainstorming a way to distinguish expansions that can and cannot include markup, I'm imagining something like POSIX shell's word expansion:

  • start with the value of modelinefmt
    • %{%val{bufname}} %opt{custom_formatted_opt} {Info}%sh{date +%H:%M} - %{%val{client}@[%val{session}]}
  • Lex the string into "expansions" and "not expansions"
    • %{%{val{bufname}}
    • %opt{custom_formatted_opt}
    • {Info}
    • %sh{date +%H:%M}
    • -
    • %{%val{client}@[%val{session}]}
  • Expand the expansion (if any) in each span
    • %val{bufname}
    • {Error}uncommitted changes
    • {Info}
    • 12:34
    • -
    • %val{client}@[%val{session}]
  • Parse each span into a display line
  • Expand the text of each display block

The point is:

  • you can prevent an expansion from being parsed as markup by wrapping it in another expansion
  • adjacent expansions can't accidentally produce markup; %({Err)%(or}x) should not produce the same result as %({Error}x)

Screwtapello avatar Jan 18 '22 04:01 Screwtapello