kakoune: update hooks

[lobre]

Description

Some hooks were removed in Kakoune, and some were added. This PR updates the list so they are aligned with the latest version of Kakoune.

Removed

mawww/kakoune@e4fb70e

• NormalBegin
• NormalEnd
• InsertBegin
• InsertEnd

mawww/kakoune@78419bc

• InsertCompletionSelect

Added

mawww/kakoune@c8839e7

• ClientCreate
• ClientClose

mawww/kakoune@47ba36c

• RegisterModified

mawww/kakoune@f2cc7bc

• User

Checklist

• Change is backwards compatible.

• Code formatted with ./format.

• Code tested through nix-shell --pure tests -A run.kakoune-{no-plugins,use-plugins,whitespace-highlighter,whitespace-highlighter-corner-cases}.

• Test cases updated/added. See example.

• Commit messages are formatted like

  {component}: {description}
  
  {long description}
  

  See CONTRIBUTING for more information and recent commit messages for examples.

[teto]

is there a way to automate that ?

[sumnerevans]

Maybe this shouldn't even be an enum?

[lobre]

All hooks are defined in this header file at the moment.

https://github.com/mawww/kakoune/blob/760a45556cfb8f50813ce8d1712a0a23f86dcf52/src/hook_manager.hh#L18

Technically, everything is doable. So I guess we could have a script that parses this file and updates hooks accordingly in home manager. But this means we become dependent on the source code implementation.

I think I am more in favour of having a string instead of an enum, as suggested by @sumnerevans. I am not sure it brings a lot of value to have an enum.

Kakoune is a project that is rather stable, so the core is not expected to change a lot, but this could still happen that hooks get changed/deleted/added from time to time.

What do you think @teto?

[teto]

I think I am more in favour of having a string instead of an enum, as suggested by @sumnerevans. I am not sure it brings a lot of value to have an enum.

looks better than a misleading enum. Something I want to do for neovim is to check the generated config after the build. I suppose we could do the same with kakoune ?

[lobre]

What do you mean exactly by "checking the generated config" @teto?

Something like launching the editor and checking if an error is returned due to a bad configuration? Or parsing the generated file to somehow analyze if it is a "correct" one? For Vim/Neovim, I guess that checking a lua file is doable, but as Kakoune uses its own configuration format (which is kind of a wrapper around bash), not sure how we can do this easily.

If you agree, I can modify the PR to instead use a plain string for those hooks. Just let me know.

Thanks.

[teto]

Something like launching the editor and checking if an error is returned due to a bad configuration?

sorry for being unclear but yeah ^ is basically what I had in mind.

[lobre]

I see, I am not sure to know how to implement that, however. For the time being, should I simply modify the PR to use a string instead of an enum then?

[teto]

Yes to using a string instead of an enum.

As for the check you could run in checkPhase kak -e "quit" with KAKOUNE_CONFIG_DIR set so that it loads the generated config. I suppose kak would return != 0 if there is an error in config

[lobre]

I am struggling to make this checkPhase working. I am trying to make the check implemented the way I stated on the attached kakoune issue. This is kind of a hack, because kakoune does not have an explicit way of checking the configuration, and I am not sure the maintainers will want to add that.

mawww/kakoune#4796 (comment)

Here is the bit that is important in the thread:

cd $(mktemp -d)
echo "colorscheme wrongtheme" > kakrc
export KAKOUNE_CONFIG_DIR=$(pwd)
kak -n -e 'try %{ source /usr/share/kak/kakrc; quit 0 } catch %{ quit 1 }'
echo $?

# results to 1

So kakoune has got a system configuration file /usr/share/kak/kakrc (or ${kakoune-unwrapped}/share/kak/kakrc in nix) which acts as the entrypoint of the program. This main configuration file is in charge of defining some basic commands such as colorscheme and it deals with the autoload of plugins, and the load of the user configuration. As a reference, here is the file in the github repo.

https://github.com/mawww/kakoune/blob/master/share/kak/kakrc

We can see that this system kakrc uses the variable ${kak_config} (which can be overridden using KAKOUNE_CONFIG_DIR) to locate and source the user configuration.

If there is a problem with configuration when the program is executed, kakoune will simply report the errors in a so-called "debug buffer", but will not exit nor return any error code. So that means we have to manually find a way to prevent the configuration from loading automatically and use kakoune’s try/catch statements to manually load and check the configuration. That is tedious but possible.

The flag -n of kakoune prevents the main kakrc file from loading. So the above script could work. It prevents the main kakrc from executing, it tries to manually source it, and if that fails, it exits the editor with the exit code 1. The only missing bit is to replace the global /usr/share/kak/kakrc (which is the global path taken from a non-nixos distribution) by the one from nix. And I am not sure to know how to do this.

So far, I have this (extract of modules/programs/kakoune.nix).

  in pkgs.writeTextFile {
    name = "kakrc";
    checkPhase = ''
      # copy the generated configuration to the current dir
      cp $target kakrc

      # make the current directory as the kakoune user configuration directory
      export KAKOUNE_CONFIG_DIR=$(pwd)

      # try to check the configuration is valid
      ${pkgs.kakoune-unwrapped}/bin/kak -n -e 'try %{ source ${pkgs.kakoune-unwrapped}/share/kak/kakrc; quit 0 } catch %{ quit 1 }'"
    '';
    text =
      (optionalString (cfg.config != null) cfgStr + "\n" + cfg.extraConfig);

But unfortunately, it seems that ${pkgs.kakoune-unwrapped}/share/kak/kakrc does not exist yet. I am not super familiar with checkPhase, but at that moment in time, I guess that the postInstall of kakoune-unwrapped as defined in https://github.com/NixOS/nixpkgs/blob/nixos-22.11/pkgs/applications/editors/kakoune/default.nix#L27 has not been executed, and so the default kakrc does not exist in this checkPhase context.

Do you have an idea on how to workaround this @teto?

And all this complexity makes me wonder about the maintainability of this configuration checker. I don’t know if it is a good idea to add something that feels that clunky to check the configuration of kakoune, as there is no official way of doing it.

[lobre]

As I was in an extensive discovery phase of kakoune those past few weeks, so I created a few PRs here and there when I saw inconsistencies. But I finally decided to go back to neovim, so I wanted to say that I will probably loose interest in this PR if it does not get merged soon.

I don't mean to hurry anything, but maybe we should simply change the hooks into a string, and keep the configuration validation for another PR in the future (as it is not as trivial as what I thought).

What do you think @teto?

[teto]

If u are losing interest, I think we can merge as is. If you have some code pending, reference the branch here so that a future kakoune user can resume from it.

[lobre]

Let’s do this then.

I have created a follow-up PR that I just closed, just for reference.

It has all the information about the WIP.

#3537

So I guess this current PR can be merged then.

Thanks a lot