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

kconifg:init #2565

Merged
merged 1 commit into from
Apr 19, 2024
Merged

kconifg:init #2565

merged 1 commit into from
Apr 19, 2024

Conversation

pasqui23
Copy link
Contributor

@pasqui23 pasqui23 commented Dec 15, 2021

Description

Fixes #607 by providing a way to programattically set kde configuration files.
However as note that any theme changed with this module requires a logout, as the proper way would be to exec plasma-apply-* and those commands seem to require a working DBus and thus #2548 for testing on my setup.

Checklist

  • Change is backwards compatible.

  • Code formatted with ./format.

  • Code tested through nix-shell --pure tests -A run.all.

  • 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.

  • If this PR adds a new module

    • Added myself as module maintainer. See example.

    • Added myself and the module files to .github/CODEOWNERS.

@pasqui23 pasqui23 requested a review from rycee as a code owner December 15, 2021 17:31
@LunNova
Copy link
Contributor

LunNova commented Dec 15, 2021

Some thoughts after trying something like this locally:

Activate after linkGeneration rather than writeBoundary so if someone removes an old home-manager config for one of these files and then sets it up this way, the old link is removed before trying to set it.
What does this script do if a file doesn't already exist, or if it can't be written to?

Ask KWin and KGlobalSettings to reload configs so they can apply immediately. Seems to work for me but not confident this reloads all configs:

      $DRY_RUN_CMD ${pkgs.libsForQt5.qt5.qttools.bin}/bin/qdbus org.kde.KWin /KWin reconfigure || echo "KWin reconfigure failed"
      # the actual values are https://github.com/KDE/plasma-workspace/blob/c97dddf20df5702eb429b37a8c10b2c2d8199d4e/kcms/kcms-common_p.h#L13
      for changeType in {0..10}; do
        $DRY_RUN_CMD ${pkgs.dbus}/bin/dbus-send --type=signal /KGlobalSettings org.kde.KGlobalSettings.notifyChange int32:$changeType int32:0 || echo "KGlobalSettings.notifyChange failed"
      done

@pasqui23
Copy link
Contributor Author

pasqui23 commented Dec 16, 2021

Some thoughts after trying something like this locally:

What does this script do if a file doesn't already exist, or if it can't be written to?

The file is created without problems

Ask KWin and KGlobalSettings to reload configs so they can apply immediately. Seems to work for me but not confident this reloads all configs:

      $DRY_RUN_CMD ${pkgs.libsForQt5.qt5.qttools.bin}/bin/qdbus org.kde.KWin /KWin reconfigure || echo "KWin reconfigure failed"
      # the actual values are https://github.com/KDE/plasma-workspace/blob/c97dddf20df5702eb429b37a8c10b2c2d8199d4e/kcms/kcms-common_p.h#L13
      for changeType in {0..10}; do
        $DRY_RUN_CMD ${pkgs.dbus}/bin/dbus-send --type=signal /KGlobalSettings org.kde.KGlobalSettings.notifyChange int32:$changeType int32:0 || echo "KGlobalSettings.notifyChange failed"
      done

That requires dbus access, not aviable in nixos module until #2548 is merged

@LunNova
Copy link
Contributor

LunNova commented Dec 16, 2021

Strange, it works for me already, tested by toggling settings and confirming they are applied immediately with that change.

@pasqui23
Copy link
Contributor Author

@LunNova If you are using home-manager from the terminal and not as a NixOS module then it should work, yes.

g=''" --group "'';
groupPortion = ''.[1:-2]|join(${g})'';
getVal = "$G|getpath($P)";
mkExecLn = ''"${w}"+.[0]+${g}+(${groupPortion})+" --key "+.[-1]+(${getVal})'';
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it support inner groups? I don't completely understand what happens here :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, all that is specifically to support inner groups

@kurnevsky
Copy link
Contributor

Can we have a setting to wait until default kde configs are created? Here I set hotkeys in khotkeysrc but expect that default hotkeys are set by kde: https://github.com/kurnevsky/nixfiles/blob/0ba02c1adf544d6f635fe9d5a50046e93fbd4955/modules/kde.nix#L211 If config is created by kwriteconfig5 kde won't set defaults I suppose.

@stale
Copy link

stale bot commented Mar 31, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Mar 31, 2022
@pasqui23
Copy link
Contributor Author

pasqui23 commented Apr 2, 2022

Still relevant

Can we have a setting to wait until default kde configs are created? Here I set hotkeys in khotkeysrc but expect that default hotkeys are set by kde: https://github.com/kurnevsky/nixfiles/blob/0ba02c1adf544d6f635fe9d5a50046e93fbd4955/modules/kde.nix#L211 If config is created by kwriteconfig5 kde won't set defaults I suppose.

kwriteconfig modifies the old configs without deleting them,so old configuration would not me modified if not opresent in the nix config

@stale stale bot removed the status: stale label Apr 18, 2022
@pasqui23 pasqui23 force-pushed the kwriteconfig branch 2 times, most recently from d6be931 to 6b39aea Compare June 19, 2022 13:05
@pasqui23 pasqui23 force-pushed the kwriteconfig branch 2 times, most recently from 6946097 to f0643ef Compare August 15, 2022 12:08
Copy link
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution and sorry for the very late review.

I have no mean to test the PR and assume you have been using it and testing it for some time.

modules/misc/qt/kconfig.nix Outdated Show resolved Hide resolved
@stale
Copy link

stale bot commented Nov 25, 2022

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Nov 25, 2022
@pasqui23 pasqui23 requested review from LunNova and kurnevsky and removed request for rycee and LunNova November 25, 2022 17:16
@pasqui23 pasqui23 requested review from LunNova and removed request for kurnevsky November 25, 2022 17:16
@stale stale bot removed the status: stale label Feb 27, 2023
@stale
Copy link

stale bot commented May 31, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label May 31, 2023
@pasqui23 pasqui23 requested review from LunNova and berbiche June 1, 2023 09:48
@stale stale bot removed the status: stale label Jun 1, 2023
@Prn-Ice
Copy link

Prn-Ice commented Aug 19, 2023

Going to poke at this to keep it alive.

Copy link

stale bot commented Nov 18, 2023

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@pasqui23
Copy link
Contributor Author

Still waiting for the review

Copy link

stale bot commented Feb 17, 2024

Thank you for your contribution! I marked this pull request as stale due to inactivity. Please read the relevant sections below before commenting.

If you are the original author of the PR

  • GitHub sometimes doesn't notify people who commented / reviewed a PR previously when you (force) push commits. If you have addressed the reviews you can officially ask for a review from those who commented to you or anyone else.
  • If it is unfinished but you plan to finish it, please mark it as a draft.
  • If you don't expect to work on it any time soon, please consider closing it with a short comment encouraging someone else to pick up your work.
  • To get things rolling again, rebase the PR against the target branch and address valid comments.

If you are not the original author of the PR

  • If you want to pick up the work on this PR, please create a new PR and indicate that it supercedes and closes this PR.

@stale stale bot added the status: stale label Feb 17, 2024
@rycee rycee force-pushed the kwriteconfig branch 2 times, most recently from e42e52e to ba5460a Compare February 18, 2024 23:02
@rycee
Copy link
Member

rycee commented Feb 18, 2024

I added a minor fixup commit. Please have a look, if it seems fine then I'll squash it and merge.

@rycee rycee added the pinned Prevent marking as stale label Feb 18, 2024
@stale stale bot removed the status: stale label Mar 11, 2024
modules/modules.nix Outdated Show resolved Hide resolved
pkgs.runCommandLocal "kwriteconfig.sh" {
passAsFile = [ "cfg" "jqScript" ];
cfg = builtins.toJSON (lib.mapAttrsRecursive toKconfVal cfg);
jqScript = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

I wrote it in nix with support of multiple inner groups without this jq sorcery, if you are interested:

kurnevsky/nixfiles@68ed3b9#diff-580825c1977fc8552d050516c2b2eb93dcb9d0ce88dfece8bba8816d576ad290R317

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, I followed your example

@pasqui23 pasqui23 force-pushed the kwriteconfig branch 2 times, most recently from f91980c to c19c242 Compare April 8, 2024 12:59
@rycee rycee merged commit 6a171bf into nix-community:master Apr 19, 2024
4 checks passed
@rycee
Copy link
Member

rycee commented Apr 19, 2024

Thanks a lot for your patience and hard work! Finally merged to master now 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned Prevent marking as stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KDE / Plasma config tracking issue
7 participants