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 indentation options to kate #88

Merged
merged 8 commits into from
Mar 24, 2024
Merged

Conversation

Asqiir
Copy link
Contributor

@Asqiir Asqiir commented Mar 19, 2024

This is the first contribution to #87 .
It allows users to configure the way kate handles indentation.

Reasoning about decisions:

I decided to put all options into config.programs.kate.editor. The editor option should contain all options that act on configuration into the rectangle where users type text in (as opposed to: the console, the project tab, the menus and sidebards).

Known limitations:

  • kate needs to be restart after switching the configuration. I could not figure out how to make kate do a hot reload on the changed katerc.
  • I would like to have the message in the assertion reference the variables in question not hard-coded by their name. I would prefer if the compilation fails if someone was to change the names of the variable but forgot to update the message.

Do you want me to put all changes into a single MR (leaving this one open), or do you prefer each change to be its own PR?

@Asqiir Asqiir mentioned this pull request Mar 19, 2024
5 tasks
Copy link
Collaborator

@magnouvean magnouvean left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my two small comments. Looks good to me otherwise assuming you have tested that the module works (I don't really use kate so I can't really test)

modules/apps/kate.nix Outdated Show resolved Hide resolved
modules/apps/kate.nix Outdated Show resolved Hide resolved
@Asqiir
Copy link
Contributor Author

Asqiir commented Mar 22, 2024

found bugs :(
(do not merge yet, I hope to fix it today)

fixed

@Asqiir Asqiir force-pushed the indentation branch 2 times, most recently from ffd3787 to 6770e93 Compare March 22, 2024 20:57
@magnouvean
Copy link
Collaborator

I added an enable option (which is in general good practice) and assigned config.programs.kate to a variable cfg (which we do throughout the project to shorten things up a bit). See my two earlier commits. Are you happy enough with this so that we can merge this now @Asqiir?

@Asqiir
Copy link
Contributor Author

Asqiir commented Mar 24, 2024

@magnouvean we can merge this imo

@magnouvean magnouvean merged commit 8a032af into nix-community:trunk Mar 24, 2024
1 check passed
@Asqiir Asqiir deleted the indentation branch March 25, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants