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

Powerdevil revamp #384

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

Powerdevil revamp #384

wants to merge 58 commits into from

Conversation

tucho
Copy link
Contributor

@tucho tucho commented Oct 14, 2024

I tried to cover all the options present in the configuration screen. I also made some changes to the code structure with the intention of making it more uniform and organized.

As for the option names, my idea was to mirror the texts of the options on the configuration screen. I think that even with longer names, it will be easier for the end user to assemble their configuration from the screens.

Please consider my contribution as a suggestion and not as an attempt to change for the sake of change. We can and should discuss these changes and, if necessary, reach a compromise in terms of nomenclature and organization.

One more thing: don't be scared by the number of commits, it's just my way of working. I suggest that you analyze the changes based on the final code.

tucho added 30 commits October 6, 2024 14:00
It is easier to see the differences when the options are all in on line.
…rnalMonitorIsConnected

It is worth noting that the semantics of this option have been inverted by the renaming.
Reformat the code so that it is "more vertical".
tucho added 25 commits October 13, 2024 14:13
To disallow the screen to dim automatically we must set
dimAutomatically.idleTimeout to "never".
Extract the general configuration options and move it close to the
function that builds the profiles options.
This assertion does not depend on the power profile, so it does not
belong to the function.
@HeitorAugustoLN
Copy link
Member

I think this mostly looks good, besides the reformatting of lists, because they are formatted like this because of nixfmt. But I still need to take a better look at it.

@magnouvean magnouvean added the 4. has: plasma module (update) Updates plasma module label Oct 15, 2024
@HeitorAugustoLN HeitorAugustoLN added the 1. scope: powerdevil Related to powerdevil module label Oct 15, 2024
@max06
Copy link

max06 commented Oct 18, 2024

Looking forward to this - the new display dimming options in kde are annoying. 👍🏼

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

Successfully merging this pull request may close these issues.

4 participants