Skip to content

Commit

Permalink
feature: subcommmand_required parameter (#170)
Browse files Browse the repository at this point in the history
* Add Infallible to __NonExhaustive enum variants

* add: subcommmand_required

* Fix a few things

- The code was not properly formatted (`cargo fmt --all`)
- There was a redundant is_subcommand value added to a bunch of things (including find_command - this PR was a breaking change and would have belonged to the `next` branch! But since is_subcommand was redundant and I removed it, this PR becomes not-breaking again (which makes sense; this kind of small feature shouldn't need breaking changes))
- The is_subcommand check was in the wrong place. The parse_invocation is really only for parsing, not for any action. If it returns an error, it should be an error in _parsing_, not in execution or usage. And it also belongs after the track_edits related checks, because if a message was edited that poise isn't even allowed to care about, it shouldn't throw an error too (this would have been another problem with the is_subcommand check in parse_invocation)

* Improve text

---------

Co-authored-by: kangalioo <jannik.a.schaper@web.de>
Co-authored-by: xtfdfr <sadorowo@gmail.com>
  • Loading branch information
3 people authored May 10, 2023
1 parent ae180fd commit 7866109
Show file tree
Hide file tree
Showing 8 changed files with 91 additions and 0 deletions.
2 changes: 2 additions & 0 deletions examples/feature_showcase/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod panic_handler;
mod parameter_attributes;
mod raw_identifiers;
mod response_with_reply;
mod subcommand_required;
mod subcommands;
mod track_edits;

Expand Down Expand Up @@ -65,6 +66,7 @@ async fn main() {
// raw_identifiers::r#move(), // Currently doesn't work (issue #170)
response_with_reply::reply(),
subcommands::parent(),
subcommand_required::parent_subcommand_required(),
track_edits::test_reuse_response(),
track_edits::add(),
],
Expand Down
34 changes: 34 additions & 0 deletions examples/feature_showcase/subcommand_required.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
use crate::{Context, Error};

/// A command with two subcommands: `child1` and `child2`
///
/// Running this function directly, without any subcommand, is only supported in prefix commands.
/// Discord doesn't permit invoking the root command of a slash command if it has subcommands.
/// This command can be invoked only with `parent child1` and `parent child2`, due to `subcommand_required` parameter.
/// If you want to allow `parent` to be invoked without subcommand, remove `subcommand_required` parameter
#[poise::command(
prefix_command,
slash_command,
subcommands("child1", "child2"),
subcommand_required
)]
// Omit 'ctx' parameter here. It is not needed, because this function will never be called.
// TODO: Add a way to remove 'ctx' parameter, when `subcommand_required` is set
pub async fn parent_subcommand_required(_: Context<'_>) -> Result<(), Error> {
// This will never be called, because `subcommand_required` parameter is set
Ok(())
}

/// A subcommand of `parent`
#[poise::command(prefix_command, slash_command)]
pub async fn child1(ctx: Context<'_>) -> Result<(), Error> {
ctx.say("You invoked the first child command!").await?;
Ok(())
}

/// Another subcommand of `parent`
#[poise::command(prefix_command, slash_command)]
pub async fn child2(ctx: Context<'_>) -> Result<(), Error> {
ctx.say("You invoked the second child command!").await?;
Ok(())
}
15 changes: 15 additions & 0 deletions macros/src/command/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub struct CommandArgs {
// if it's actually irrational, the inconsistency should be fixed)
subcommands: crate::util::List<syn::Path>,
aliases: crate::util::List<String>,
subcommand_required: bool,
invoke_on_edit: bool,
reuse_response: bool,
track_deletion: bool,
Expand Down Expand Up @@ -146,6 +147,18 @@ pub fn command(
return Err(syn::Error::new(proc_macro2::Span::call_site(), err_msg).into());
}

// If subcommand_required is set to true, then the command cannot have any arguments
if args.subcommand_required && function.sig.inputs.len() > 1 {
let err_msg = "subcommand_required is set to true, but the command has arguments";
return Err(syn::Error::new(proc_macro2::Span::call_site(), err_msg).into());
}

// If subcommand_required is set to true, then the command must have at least one subcommand
if args.subcommand_required && args.subcommands.0.is_empty() {
let err_msg = "subcommand_required is set to true, but the command has no subcommands";
return Err(syn::Error::new(proc_macro2::Span::call_site(), err_msg).into());
}

// Collect argument names/types/attributes to insert into generated function
let mut parameters = Vec::new();
for command_param in function.sig.inputs.iter_mut().skip(1) {
Expand Down Expand Up @@ -263,6 +276,7 @@ fn generate_command(mut inv: Invocation) -> Result<proc_macro2::TokenStream, dar
let default_member_permissions = &inv.default_member_permissions;
let required_permissions = &inv.required_permissions;
let required_bot_permissions = &inv.required_bot_permissions;
let subcommand_required = inv.args.subcommand_required;
let owners_only = inv.args.owners_only;
let guild_only = inv.args.guild_only;
let dm_only = inv.args.dm_only;
Expand Down Expand Up @@ -318,6 +332,7 @@ fn generate_command(mut inv: Invocation) -> Result<proc_macro2::TokenStream, dar
context_menu_action: #context_menu_action,

subcommands: vec![ #( #subcommands() ),* ],
subcommand_required: #subcommand_required,
name: #command_name.to_string(),
name_localizations: #name_localizations,
qualified_name: String::from(#command_name), // properly filled in later by Framework
Expand Down
1 change: 1 addition & 0 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ for example for command-specific help (i.e. `~help command_name`). Escape newlin
- `guild_only`: Restricts command callers to only run on a guild
- `dm_only`: Restricts command callers to only run on a DM
- `nsfw_only`: Restricts command callers to only run on a NSFW channel
- `subcommand_required`: Requires a subcommand to be specified (prefix only)
- `check`: Path to a function which is invoked for every invocation. If the function returns false, the command is not executed (can be used multiple times)
## Help-related arguments
Expand Down
13 changes: 13 additions & 0 deletions src/builtins/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,19 @@ pub async fn on_error<U, E: std::fmt::Display + std::fmt::Debug>(
eprintln!("An error occured in a command: {}", error);
ctx.say(error).await?;
}
crate::FrameworkError::SubcommandRequired { ctx } => {
let subcommands = ctx
.command()
.subcommands
.iter()
.map(|s| &*s.name)
.collect::<Vec<_>>();
let response = format!(
"You must specify one of the following subcommands: {}",
subcommands.join(", ")
);
ctx.send(|b| b.content(response).ephemeral(true)).await?;
}
crate::FrameworkError::CommandPanic { ctx, payload: _ } => {
// Not showing the payload to the user because it may contain sensitive info
ctx.send(|b| {
Expand Down
9 changes: 9 additions & 0 deletions src/dispatch/prefix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ pub async fn parse_invocation<'a, U: Send + Sync, E>(
invocation_data,
trigger,
})?;

let action = match command.prefix_action {
Some(x) => x,
// This command doesn't have a prefix implementation
Expand Down Expand Up @@ -294,6 +295,14 @@ pub async fn run_invocation<U, E>(
return Ok(());
}

if ctx.command.subcommand_required {
// None of this command's subcommands were invoked, or else we'd have the subcommand in
// ctx.command and not the parent command
return Err(crate::FrameworkError::SubcommandRequired {
ctx: crate::Context::Prefix(ctx),
});
}

super::common::check_permissions_and_cooldown(ctx.into()).await?;

// Typing is broadcasted as long as this object is alive
Expand Down
2 changes: 2 additions & 0 deletions src/structs/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub struct Command<U, E> {
// ============= Command type agnostic data
/// Subcommands of this command, if any
pub subcommands: Vec<Command<U, E>>,
/// Require a subcommand to be invoked
pub subcommand_required: bool,
/// Main name of the command. Aliases (prefix-only) can be set in [`Self::aliases`].
pub name: String,
/// Localized names with locale string as the key (slash-only)
Expand Down
15 changes: 15 additions & 0 deletions src/structs/framework_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ pub enum FrameworkError<'a, U, E> {
/// General context
ctx: crate::Context<'a, U, E>,
},
/// Command was invoked without specifying a subcommand, but the command has `subcommand_required` set
SubcommandRequired {
/// General context
ctx: crate::Context<'a, U, E>,
},
/// Panic occured at any phase of command execution after constructing the `crate::Context`.
///
/// This feature is intended as a last-resort safeguard to gracefully print an error message to
Expand Down Expand Up @@ -187,6 +192,7 @@ impl<'a, U, E> FrameworkError<'a, U, E> {
Self::Setup { ctx, .. } => ctx,
Self::EventHandler { ctx, .. } => ctx,
Self::Command { ctx, .. } => ctx.serenity_context(),
Self::SubcommandRequired { ctx } => ctx.serenity_context(),
Self::CommandPanic { ctx, .. } => ctx.serenity_context(),
Self::ArgumentParse { ctx, .. } => ctx.serenity_context(),
Self::CommandStructureMismatch { ctx, .. } => ctx.serenity_context,
Expand All @@ -209,6 +215,7 @@ impl<'a, U, E> FrameworkError<'a, U, E> {
pub fn ctx(&self) -> Option<crate::Context<'a, U, E>> {
Some(match *self {
Self::Command { ctx, .. } => ctx,
Self::SubcommandRequired { ctx } => ctx,
Self::CommandPanic { ctx, .. } => ctx,
Self::ArgumentParse { ctx, .. } => ctx,
Self::CommandStructureMismatch { ctx, .. } => crate::Context::Application(ctx),
Expand Down Expand Up @@ -264,6 +271,13 @@ impl<U, E: std::fmt::Display> std::fmt::Display for FrameworkError<'_, U, E> {
Self::Command { error: _, ctx } => {
write!(f, "error in command `{}`", full_command_name!(ctx))
}
Self::SubcommandRequired { ctx } => {
write!(
f,
"expected subcommand for command `{}`",
full_command_name!(ctx)
)
}
Self::CommandPanic { ctx, payload: _ } => {
write!(f, "panic in command `{}`", full_command_name!(ctx))
}
Expand Down Expand Up @@ -365,6 +379,7 @@ impl<'a, U: std::fmt::Debug, E: std::error::Error + 'static> std::error::Error
Self::Setup { error, .. } => Some(error),
Self::EventHandler { error, .. } => Some(error),
Self::Command { error, .. } => Some(error),
Self::SubcommandRequired { .. } => None,
Self::CommandPanic { .. } => None,
Self::ArgumentParse { error, .. } => Some(&**error),
Self::CommandStructureMismatch { .. } => None,
Expand Down

0 comments on commit 7866109

Please sign in to comment.