feat: Add powermenu plugin #203
Conversation
|
Hi @Kirottu, now that you're back, may I ask you for some feedback on this? Is this plugin of any interest to you? I just want to know whether it make sense for me to update the PR. |
|
Sorry for the late reply, a rebase is needed for me to have a look at this. Though I do think having a powermenu plugin as a part of the core plugins would probably be a good idea. |
bcyran
force-pushed
the
powermenu-plugin
branch
from
4ef05f7 to
e83224f
Compare
|
No worries, thanks for the answer. I rebased and updated the PR. |
Kirottu
left a comment
Kirottu
left a comment
There was a problem hiding this comment.
Alrighty, sorry again for the long delay but here's my review of it. I proposed some architectural refactors to make the plugin more extensible by end users and more maintainable if actions need to change for whatever reason. Also traditional code style bikeshedding, as is customary.
| power_action_config | ||
| }; | ||
|
|
||
| let action_result = execute_power_action(power_action_config); |
There was a problem hiding this comment.
This should be implemented in PowerActionConfig/PowerAction as per earlier comment.
| fn get_matches(input: RString, state: &State) -> RVec<Match> { | ||
| if input.is_empty() { | ||
| vec![] | ||
| } else if let Some(ref error_message) = state.error_message { |
There was a problem hiding this comment.
Minor nitpick, I prefer using let Some(error_message) = &state.error_message instead of using the ref keyword.
| hibernate: PowerActionConfig, | ||
| } | ||
|
|
||
| impl Config { |
There was a problem hiding this comment.
For the default power actions, there should probably be a list of possible commands to run to accomodate different init systems. Or init agnostic power management commands (if applicable) could be used.
| .into() | ||
| } | ||
|
|
||
| fn get_error_matches(error_message: &str) -> Vec<Match> { |
There was a problem hiding this comment.
Not sure how necessary error handling in a graphical way is for the plugin, I think outputting errors in stderr with eprintln! would be sufficient.
|
|
||
| #[derive(Clone, Copy, IntoPrimitive, TryFromPrimitive)] | ||
| #[repr(u64)] | ||
| pub enum PowerAction { |
There was a problem hiding this comment.
Power actions could probably be represented as a configurable list instead of a hardcoded enum, so that adding new actions and/or removing unnecessary ones is easily configurable and extensible. So in effect PowerAction and PowerActionConfig could be merged into something like this:
struct PowerAction {
title: String,
desc: String,
icon: String,
// List of commands to try to perform the action, to accomodate other inits
command: Vec<String>,
confirm: bool,
}The default config should of course contain all of the power options that are currently available in the code.
There was a problem hiding this comment.
I actually intended to implement a generic "command runner" plugin some day. I feel like after applying this suggestion, this plugin would be pretty much just that, but with power management commands included by default.
So, maybe we could drop the powermenu plugin and make more generic commandrunner. Power management actions could be then either:
- just an example config in the readme, or,
- embedded in the plugin and enabled using some predefined option.
What do you think?
There was a problem hiding this comment.
Yea that would basically turn this into a generic command runner, so I think actions is a better name to describe what it does. I do think power management actions are a good default to have in the default config and thus should also be in the README to hopefully reduce any possible confusion (and also to provide easy examples).
| .map(|score| (action, score)) | ||
| }) | ||
| .collect::<Vec<_>>(); | ||
| matches_with_scores.sort_by_key(|(_action, score)| *score); |
There was a problem hiding this comment.
The sorting seems to be inverted, since poweroff/shutdown is the last one with this query:
| @@ -0,0 +1,15 @@ | |||
| [package] | |||
| name = "powermenu" | |||
| version = "0.1.0" | |||
There was a problem hiding this comment.
Version should be the same as the other versions in the repo right now, so 25.12.0.
2 participants
A simple plugin I find very helpful and use everyday. Here's an example of how it looks in action:
I get that anyrun is in maintenance mode - I leave it to the maintainers to decide whether this should or should not be merged.