Add: Config file

[a-kenji]

closes #169

This is not 100% finished, but it is ready to get more input - especially now that the default keybinding changes have gone through.

• add config option: --config (should we use a short option for it too?)
• add config struct and intermediate structs for deserialisation

I started writing tests in the integration location, but it seemed maybe that is not the best location for these specific tests. I would need every single field and every function public for that to work. For now I put the tests under /ut/*_test.rs (for unit_tests) for now, would be happy about more input on a good location for them. I haven't finished writing all that I wanted to write for them yet.

Starting zellij without an option:

• looks for config in default location -> logs errors to log.txt
  Should Starting zellij with --config file fail and log the error to stderr?
  Or also log to file and use defaults?

Also the format changed now, because my initial idea of having a vec as a possible key in yaml is really uncomfortable to write and too error prone imo, so I settled on following format:

---
keybinds:
    normal:
        - action: [GoToTab: 1,]
          key: [F: 1,]
        - action: [GoToTab: 2,]
          key: [F: 2,]
        - action: [GoToTab: 3,]
          key: [F: 3,]
        - action: [GoToTab: 4,]
          key: [F: 4,]
        - action: [NewTab, NewTab,]
          key: [F: 5, F:6, F:7,]

That way it is easy to specify multiple keys in succession , as well as bind multiple different keys to these actions at the same time.

[imsnif]

First of all, very nice work @a-kenji !! And good call on getting early feedback on this - a lot of this feature is about user experience and I agree this is a good thing to discuss before implementing.

I'm mostly going to comment on the UX side of things (as well as address your questions) for now:

1. --config should definitely have a short option if possible
2. I think the only time the default configuration should be loaded is if the --config flag is not provided. If it's provided and there's some sort of error, the app should exit with an error and print out the error (in the most user friendly way possible).
3. I don't know if this is out of scope for this PR, but how about including a configuration option that would allow users to clear the default keybindings for a given mode? Maybe something like delete_default_keybindings: true under normal? What do you think?
4. Somewhat related to point 3, how much trouble do you think would be to provide an option that would dump the default configuration to a file? Then the user can edit that file to easily change what they want and also have a comprehensive example that we don't have to maintain?
5. About unit tests: I think it's a good call not to have them in integration. My preference would be to have them inline in the same file. But if you think it will make it too long and unwieldy we can definitely have them in a separate folder/file. In that case, let's call the folder unit if it's the sibling of integration.
6. The help section in the README is really great and a nice touch. For our documentation, I think it would be very necessary to also have a full list of actions and possible keybindings (even if only as a link to the Termion documentation). Ideally this should be something we can auto-generate from the code. Maybe it's out of scope for this though. What do you think?
7. We should also adjust the Help output for the custom keybindings. Maybe we can combine this with the auto-generation from point 6. But in any case, if a user adds/deletes/overrides keybindings - it should appear in the status bar as well.

Looking forward to iterate on this more and merge it. It's a super important feature and I feel it's in good hands.

[a-kenji]

Thanks for the feedback!

To:

1. I will add -c for a short option then for now, if that makes sense.
2. Yes, that makes sense. So with the --config flag, if there is an error we exit and print the error to the terminal, should we still log it to the log.txt? And no default is loaded in that case. I think we will need an option then to also tell zellij to not load a config file? Maybe --config --no-config or something similar?
3. I thought about that too, I would think that is out of this scope, but I would be happy to work on it. I think it could be valuable to only override part of the default keybindings. My current idea is something like unbind: [] and then a list of the keys one wants to unbind under a mode. And have a shorthand for unbinding every single keybinding, for example all. Something in that direction is definitely needed.
4. I think that would be awesome! I think it wouldn't be too hard to dump the defaults, but to make it sense could be quite some iteration. So id say out of scope, like 3. I would be happy to work on it.
5. Yes, that makes a lot of sense. I will name the folder to unit if I end up using it.
6. That is a good Idea, I will link the termion documentation for now. I agree it would be awesome to generate the actions and their description - I see that related to 4.
7. Good catch! I will update the help. I would also say that the status bar is a separate issue at this point, but yes that needs to be addressed.

[imsnif]

2. Yes, that makes sense. So with the `--config` flag, if there is an error we exit and print the error to the terminal, should we still log it to the log.txt? And no default is loaded in that case. I think we will need an option then to also tell zellij to not load a config file? Maybe `--config --no-config` or something similar?

log.txt should just be for debugging... any other instances in the codebase are remnants of a less stable time :)
About no-config: Isn't the default running with no config file? Or am I misunderstanding something?

3. I thought about that too, I would think that is out of this scope, but I would be happy to work on it. I think it could be valuable to only override part of the default keybindings. My current idea is something like `unbind: []` and then a list of the keys one wants to unbind under a mode. And have a shorthand for unbinding every single keybinding, for example `all`. Something in that direction is definitely needed.

We can talk about it when it's relevant, but IMO it would be better to unbind everything rather than specifying what you'd like to unbind. Otherwise config files will have to be updated every time we change the default keys.

4. I think that would be awesome! I think it wouldn't be too hard to dump the defaults, but to make it sense could be quite some iteration. So id say out of scope, like 3. I would be happy to work on it.

Sounds good.

6. That is a good Idea, I will link the termion documentation for now. I agree it would be awesome to generate the actions and their description - I see that related to 4.

Cool.

7. Good catch! I will update the help. I would also say that the status bar is a separate issue at this point, but yes that needs to be addressed.

I think as soon as you update the Help the status bar should be taken care of (maybe with some minor bugs that can be dealt with separately).

[a-kenji]

@imsnif,
I believe this is ready to be reviewed.

log.txt should just be for debugging... any other instances in the codebase are remnants of a less stable time :)

Ah, good to know! Is there a "user" facing logging system planned already? Or is that all the -debug flag?

About no-config: Isn't the default running with no config file? Or am I misunderstanding something?

Currently I put it as a flag --clean under config

Looking for default config file:
found -> use default config file
found, but error -> exit
if not found -> use default configuration
--clean -> use default configuration

We can talk about it when it's relevant, but IMO it would be better to unbind everything rather than specifying what you'd like to unbind. Otherwise config files will have to be updated every time we change the default keys.

I will open an issue for that!

I will also open an issue for the status bar.

[khs26]

@imsnif had suggested I take a look at this, since I'd wanted to do this before when I had time. I have to say that I think it's great and cleaner than I think I'd have come up with. :)

The main thing I wondered about are future enhancements (and what you thought about them):

• Do one or both of:
  • Provide more convenient aliases for some of the commands (e.g. not using camelcase, or being a bit more permissive)
  • Actually properly document our actions struct, so that we can point users at the auto-generated docs for "things you can map in a config file"
• Defining new modes via the config file (and keybinds to switch to them)
• Dumping and storing the existing defaults as a config file rather than hardcoding them in the Rust code. I think others may have objections to this.
• Defining macros to send keystrokes too (e.g. [NewTab, WriteInput: "top", PreviousTab,] to run top in a new tab)

[a-kenji]

Thank you @khs26 for the feedback!

Provide more convenient aliases for some of the commands (e.g. not using camelcase, or being a bit more permissive)

I agree, there should be something done in that regard.
I thought about not using camelcase, but I don't know what a good alias would be, or should it be an alias?
I am happy to open an issue to see how we handle that in the future, I also think empty enum variants should be handled better in the future.

Actually properly document our actions struct, so that we can point users at the auto-generated docs for "things you can map in a config file"

That is a good idea!

Defining new modes via the config file (and keybinds to switch to them)

I think it is awesome, I thought about adding yet another default mode in order to be able to mimick tmux prefix keys. But allowing anyone to add a new mode, if they please is a more elegant way imo.

Dumping and storing the existing defaults as a config file rather than hardcoding them in the Rust code. I think others may have objections to this.

I would also have objections to that in its current state. Maybe not in the future. I think we try to get some good defaults for people so they can just start. It might make it harder for example if the config file is somehow wrongly formatted and they just want to work - but can't even get the defaults to work.

Defining macros to send keystrokes too (e.g. [NewTab, WriteInput: "top", PreviousTab,] to run top in a new tab)

It is kind of planned I think, I think it would be a very welcome addition.
This is an early draft of that feature: #169 (comment) .
It would need to look a bit different now.