Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for the kakoune text editor #637

Closed
wants to merge 9 commits into from

Conversation

@loewenheim
Copy link
Contributor

commented Mar 25, 2019

This is my first contribution to home-manager, so I’m not sure about some of the things I’ve done. In particular, I’m not sure about how to handle the following issue: The tabstop option, for instance, is set to 8 by default in kakoune and this value is assumed if not overriden in kakrc. Should I therefore:

  • Make a nix option of type int with default 8 and always emit it to kakrc?
  • Make a nix option of type int with default 8 and only emit it to kakrc if it differs from the default?
  • Make a nix option of type null or int with default null and only emit it to kakrc if it isn’t null?

Right now I’ve chosen the second option, was that correct?.

@sboosali

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

@rycee

This comment has been minimized.

Copy link
Owner

commented Mar 26, 2019

Both option 1 and 3 are OK with me. Home Manager already contain both these techniques.

I sort of am partial to 1 since it leads to nicer documentation and it is clearer for the user which value will be used in the end. But I think some others disagree with me on this. So I think as the module writer you can choose 🙂

(as long as it's not option 2)

type = types.ints.unsigned;
default = 8;
description = ''
The width of a tab

This comment has been minimized.

Copy link
@rycee

rycee Mar 26, 2019

Owner

All option description should end in a period. I.e., here it should be The width of a tab..

@loewenheim

This comment has been minimized.

Copy link
Contributor Author

commented Mar 28, 2019

I’ve now changed most of the options to be in accordance with variant 3 above. It didn’t seem sensible to make indentWithTabs and incrementalSearch have type nullOr bool, though. Is that acceptable?

@loewenheim

This comment has been minimized.

Copy link
Contributor Author

commented Mar 30, 2019

How can I check the documentation that I’ve written? I can’t find a way to compile it.

@rycee

This comment has been minimized.

Copy link
Owner

commented Apr 3, 2019

Sorry for the delay! The easiest way to see the documentation is to use home-manager switch using your repo clone. Then man home-configuration.nix will contain your changes. You can also set

manual.html.enable = true;

in your Home Manager configuration and after home-manager switch you should be able to run

$ home-manager-help

to open the HTML-manual in a browser.

@loewenheim

This comment has been minimized.

Copy link
Contributor Author

commented Apr 4, 2019

Thank you very much for the information. There were definitely some issues left with the documentation ;)

config = mkIf cfg.enable(mkMerge [
{
home.packages = [ pkgs.kakoune ];
xdg.configFile."kak/kakrc-test".source = configFile;

This comment has been minimized.

Copy link
@teto

teto Jun 5, 2019

Collaborator

fix the filename and we can merge since previous reviewers seem ok with the module.

@loewenheim loewenheim force-pushed the loewenheim:kakoune branch from 55a70bb to e5f26e0 Jun 5, 2019

@rycee

This comment has been minimized.

Copy link
Owner

commented Jul 17, 2019

Thanks! Rebased to master in 7d68c46.

Note, I made a few minor changes and reformattings. Hopefully nothing broke 🙂

@rycee rycee closed this Jul 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.