Fix debug noise from failed windowing detection

[arachsys]

To detect the current windowing system, rc/windowing/detection.kak tries to load each supported module in a try/catch chain. If no windowing system is available, the error from the last module in the list ends up uncaught and dumped into the debug buffer.

Fix the chain so it suppresses the failure message from the module listed last as well as the previous ones.

It's easy to miss this stray error message as it only happens if you run kak on a machine without any windowing system and without screen/tmux.

[Screwtapello]

If no windowing system is available, I think there should be some record in the debug log that something is missing, and maybe a suggestion of what to do about it. Windowing is very useful, and while Vim and Emacs (and many other editors) have it built-in, Kakoune's policy has always been to let plugins implement that functionality. If no available plugins work, and there's nothing in the debug log, somebody's going to have a frustrating day figuring out why they can't :new or why some other plugin explodes because the :new or :terminal aliases aren't defined.

[arachsys]

If no windowing system is available, I think there should be some record in the debug log that something is missing, and maybe a suggestion of what to do about it.

Perhaps the right thing would be an explicit fail 'No windowing system found' in the final catch rather than a nop, effectively replacing the current internal error about DISPLAY from rc/windowing/x11.kak?

Advice about :doc TOPIC in the message would be helpful too, I agree, but as far as I know there's no coverage of the windowing system scripts in those asciidocs?

[Screwtapello]

Even without linking to documentation, the error message could say something like "run Kakoune inside tmux or GNU Screen for windowing support".

[mawww]

I dont think we should fail on load time, but detection.kak could fallback to a terminal alias that does fail with such a message.

Not having windowing support is fine, it would happen if you run Kakoune from the linux console for example. I would not want a scary error message on startup in that case.

[arachsys]

@mawww That makes a lot of sense. (To be honest, I was a bit hesitant to replace one annoying startup error with another one!)

I've updated the pull request with an additional patch that provides a default implementation for terminal and focus with a clearer error message that :new etc only work when Kakoune is run under a windowing system. Does that look sensible?

[arachsys]

Dropping pull request as it's been inactive for eight months.

[krobelus]

I think this is a good change.

define-command missing-windowing-system -params .. -docstring
'Placeholder implementation for the focus and terminal aliases' %{ fail This command only works inside a windowing system: $opt{windowing_modules} }

This command should have the -hidden flag, else it will be suggested in the command prompt. We usually namespace commands so if we keep the separate command, it should be like windowing-missing.

Also $opt{windowing_modules} will fail to expand. I tested it with

env -u DISPLAY -u WAYLAND_DISPLAY kak

in a terminal that doesn't have special windowing support.

Also we usually start error messages with focus: (or terminal: ) so the user knows where an error is coming from. Running a missing command shows a helpful stack trace, but fail doesn't show any of that. So we should show the name of the missing command because it might be run from a mapping / another command (Perhaps we should hack Kakoune to make fail show the entire stack trace in the debug buffer?).

Actually, missing-windowing-system is not needed most of the time. So how about we only define it if needed?

Here's my suggestion that should address all points, what do you think?

hook -group windowing global KakBegin .* %{
    evaluate-commands %[
        try %{
    ] %sh[
        set -- ${kak_opt_windowing_modules}
        if [ $# -gt 0 ]; then
            printf "require-module %s } catch %%{ " "$@"
        fi
    ] %[
            define-command -hidden focus %{
                fail "focus: this command only works inside a windowing system like %opt{windowing_modules}"
            }
            define-command -hidden terminal %{
                fail "terminal: this command only works inside a windowing system like %opt{windowing_modules}"
            }
        }
    ]
}

[arachsys]

Hi Johannes, I agree my actual fix in https://github.com/mawww/kakoune/pull/4469/commits/fc5beff97feec8b79ab25a8978cb65ed1c04e73f is worthwhile. I've kept it in my own kakoune build recipe along with a handful of other local changes. Please do feel free to run with this if you like. I would be delighted to see it fixed upstream one day so I don't need to carry a local patch to avoid these crashes in the debug log.

Personally, I don't bother applying my second patch (despite having written it on request) because I see no benefit from the code expenditure. To my mind, it just wraps an already clear error message, and I'm not keen on runtime advertisements for other software. Ironically I'd probably be patching that message out locally in my cosmetic.diff if it had been merged!

FWIW the second patch did work on a non-windowing system when I wrote it, with $opt{windowing_modules} expanding correctly. (I remember looking at the message and mulling over whether I could make it prettier by comma-separating the items without adding complexity.) However, that was eight months ago so I can believe it no longer works applied to current master.

In terms of implementation, this code runs at every editor startup. Is the fork-and-exec of a shell worthwhile in that path just to conditionally hide a command definition, where it adds to the cost of every kak invocation? Purely personal opinion again, but I always feel anything like a fork+exec I can move out of the startup path so it only happens when a user explicitly uses a command is a win. The 2021.11.08 release spawns 11 shells at startup for me; I haven't checked recently to see if current master has improved on that or got more expensive.

Unconditionally defining the command might actually be a lot cheaper and cleaner?