Nukesor / pueue

:stars: Manage your shell commands.
Apache License 2.0
5k stars 132 forks source link

Accept `SIGINT`, `INT`, and `int` along with `sigint` as a vallid signal name. #455

Closed GrigorenkoPV closed 1 year ago

GrigorenkoPV commented 1 year ago

A detailed description of the feature you would like to see added.

For the -s/--signal parameter of the pueue kill command, it would be cool to be able to use signals' names both in lowercase & uppercase as well as both with the "SIG" prefix & without it.

Explain your usecase of the requested feature

An actual snippet from my terminal just now. I got "lucky" to correctly guess the format only on the 4th attempt.pueu

$ pueue kill -s INT 0
error: invalid value 'INT' for '--signal <SIGNAL>': Matching variant not found

For more information, try '--help'.
2 $ pueue kill -s SIGINT 0
error: invalid value 'SIGINT' for '--signal <SIGNAL>': Matching variant not found

For more information, try '--help'.
2 $ pueue kill -s int 0
error: invalid value 'int' for '--signal <SIGNAL>': Matching variant not found

For more information, try '--help'.
2 $ pueue kill -s sigint 0
Tasks are being killed: 0
$

At that point I was starting to get worried that I would have to pass the numerical value for the signal, which I don't remember for SIGINT. 😅

Alternatives

No response

Additional context

No response

Nukesor commented 1 year ago

Could you check if the linked PR covers all variations :)?

GrigorenkoPV commented 1 year ago

Could you check if the linked PR covers all variations :)?

Well, potentially "SigInt" is not covered, but that's kind of ridiculous. However, makes me wonder if there's a way to make the comparison case-insensitive.

I've taken a look at strum docs, there's an ascii_case_insensitive thing, which looks like it can help to get rid of

  serialize = "SigKill",
  serialize = "sigkill",
  serialize = "SIGKILL",

boilerplate.

And this seems to suggest that ascii_case_insensitive can be applied to the entire enum at once?

GrigorenkoPV commented 1 year ago
diff --git a/pueue_lib/src/network/message.rs b/pueue_lib/src/network/message.rs
index e65c826..2d3c157 100644
--- a/pueue_lib/src/network/message.rs
+++ b/pueue_lib/src/network/message.rs
@@ -187,16 +187,17 @@ impl_into_message!(PauseMessage, Message::Pause);
 /// This is also needed for usage in clap, since nix's Signal doesn't implement [Display] and
 /// [std::str::FromStr].
 #[derive(PartialEq, Eq, Clone, Debug, Deserialize, Serialize, Display, EnumString)]
+#[strum(ascii_case_insensitive)]
 pub enum Signal {
-    #[strum(serialize = "SigInt", serialize = "sigint", serialize = "2")]
+    #[strum(serialize = "int", serialize = "sigint", serialize = "2")]
     SigInt,
-    #[strum(serialize = "SigKill", serialize = "sigkill", serialize = "9")]
+    #[strum(serialize = "kill", serialize = "sigkill", serialize = "9")]
     SigKill,
-    #[strum(serialize = "SigTerm", serialize = "sigterm", serialize = "15")]
+    #[strum(serialize = "term", serialize = "sigterm", serialize = "15")]
     SigTerm,
-    #[strum(serialize = "SigCont", serialize = "sigcont", serialize = "18")]
+    #[strum(serialize = "cont", serialize = "sigcont", serialize = "18")]
     SigCont,
-    #[strum(serialize = "SigStop", serialize = "sigstop", serialize = "19")]
+    #[strum(serialize = "stop", serialize = "sigstop", serialize = "19")]
     SigStop,
 }

Something like this

Nukesor commented 1 year ago

Nice, good catch.

I somehow expected that the case_insensitive option didn't work in combination with custom serialize attributes, but I turned out to be wrong :))