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 KXMLGUI support and add konsole options for toolbar customization #362

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

HeitorAugustoLN
Copy link
Member

Closes #327

This pull request adds support for KXMLGUI configurations, adding support for Konsole shortcuts and toolbar. Additionally, the modified files have been formatted using nixfmt-rfc-style, aligning with the upcoming adoption of this formatter as the default for Nix. Since nixpkgs-fmt has been archived in favor of nixfmt-rfc-style, I think we should gradually apply this formatting to other files across the project. Thoughts?

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.

The code looks good and I've done some very limited testing and the output files look as expected.

I think reformatting with nixfmt-rfc-style makes a lot of sense. We could do this gradually every time we modify the code, or we could do it all in one pr. I think both options are fine, but I'd maybe prefer to do it all in one pr.

Also we should perhaps add kxmlgui5 to defaultResetFiles so they are reset when using overrideConfig. This will require some minor changes though as it's located in a separate directory, but still shouldn't be a lot of work.

Comment on lines +147 to +159
konsole = lib.mkOption {
type = lib.types.nullOr kxmlguiType;
default = null;
description = ''
The toolbar of Konsole.
'';
};
session = lib.mkOption {
type = lib.types.nullOr kxmlguiType;
default = null;
description = ''
The toolbar of Konsole sessions.
'';
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some example properties could maybe be good here

Comment on lines +107 to +110
menus = lib.mkOption {
type = lib.types.listOf menuType;
description = "The menus of the menu bar.";
};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this not just be a submodule of listOf itemType, so instead of having something like

menus = [
  {
    name = "name";
    items = ["item1" ...];
  }
]

you could have

menus."name" = ["item1" ...];

@HeitorAugustoLN
Copy link
Member Author

I think the current approach is not that good, I think I will implement something in a script for it instead.

@magnouvean
Copy link
Collaborator

I see that. The code would maybe be better and more extensible as well. The implementation could be very similar to the write-config script.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. type: enhancement New feature or request for feature 1. scope: konsole Related to konsole module 4. has: kde app module (update) Updates KDE app module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

adding kxmlgui5 support?
2 participants