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

Warn on Unhandled commands #1533

Merged
merged 6 commits into from
Jan 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ You can find its changes [documented below](#070---2021-01-01).

### Changed

- Warn on unhandled Commands ([#1533] by [@Maan2003])

### Deprecated

### Removed
Expand Down Expand Up @@ -603,6 +605,7 @@ Last release without a changelog :(
[#1523]: https://github.com/linebender/druid/pull/1523
[#1526]: https://github.com/linebender/druid/pull/1526
[#1532]: https://github.com/linebender/druid/pull/1532
[#1533]: https://github.com/linebender/druid/pull/1533
[#1534]: https://github.com/linebender/druid/pull/1534
[#1254]: https://github.com/linebender/druid/pull/1254

Expand Down
57 changes: 49 additions & 8 deletions druid/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,18 @@ use crate::{WidgetId, WindowId};
/// The identity of a [`Selector`].
///
/// [`Selector`]: struct.Selector.html
pub(crate) type SelectorSymbol = &'static str;
#[derive(Copy, Clone, PartialEq, Eq)]
pub(crate) struct SelectorSymbol {
str: &'static str,
must_use: bool,
}

impl std::fmt::Debug for SelectorSymbol {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let must_use = if self.must_use { " (must_use)" } else { "" };
write!(f, "{}{}", self.str, must_use)
}
}

/// An identifier for a particular command.
///
Expand Down Expand Up @@ -329,8 +340,25 @@ impl Selector<()> {

impl<T> Selector<T> {
/// Create a new `Selector` with the given string.
pub const fn new(s: &'static str) -> Selector<T> {
Selector(s, PhantomData)
pub const fn new(str: &'static str) -> Selector<T> {
Selector(
SelectorSymbol {
str,
must_use: false,
},
PhantomData,
)
}

/// Create a `Selector` that must be used.
pub const fn must_use(str: &'static str) -> Selector<T> {
Selector(
SelectorSymbol {
str,
must_use: true,
},
PhantomData,
)
}

/// Returns the `SelectorSymbol` identifying this `Selector`.
Expand Down Expand Up @@ -384,6 +412,11 @@ impl Command {
.default_to(Target::Global)
}

/// Checks if this command must be used.
pub fn must_be_used(&self) -> bool {
self.symbol.must_use
}

/// A helper method for creating a `Notification` from a `Command`.
///
/// This is slightly icky; it lets us do `SOME_SELECTOR.with(SOME_PAYLOAD)`
Expand All @@ -408,6 +441,14 @@ impl Command {
self
}

/// Make the `Command` must use.
///
/// this will log warning if this `Command` is not handled.
pub fn must_use(mut self, must_use: bool) -> Self {
self.symbol.must_use = must_use;
self
}

/// Set the correct default target when target is `Auto`.
pub(crate) fn default_to(mut self, target: Target) -> Self {
self.target.default(target);
Expand Down Expand Up @@ -446,7 +487,7 @@ impl Command {
if self.symbol == selector.symbol() {
Some(self.payload.downcast_ref().unwrap_or_else(|| {
panic!(
"The selector \"{}\" exists twice with different types. See druid::Command::get for more information",
"The selector {:?} exists twice with different types. See druid::Command::get for more information",
selector.symbol()
);
}))
Expand All @@ -472,7 +513,7 @@ impl Command {
pub fn get_unchecked<T: Any>(&self, selector: Selector<T>) -> &T {
self.get(selector).unwrap_or_else(|| {
panic!(
"Expected selector \"{}\" but the command was \"{}\".",
"Expected selector {:?} but the command was {:?}.",
selector.symbol(),
self.symbol
)
Expand All @@ -499,7 +540,7 @@ impl Notification {
if self.symbol == selector.symbol() {
Some(self.payload.downcast_ref().unwrap_or_else(|| {
panic!(
"The selector \"{}\" exists twice with different types. \
"The selector {:?} exists twice with different types. \
See druid::Command::get for more information",
selector.symbol()
);
Expand Down Expand Up @@ -541,7 +582,7 @@ impl From<Selector> for Command {

impl<T> std::fmt::Display for Selector<T> {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(f, "Selector(\"{}\", {})", self.0, any::type_name::<T>())
write!(f, "Selector({:?}, {})", self.0, any::type_name::<T>())
}
}

Expand Down Expand Up @@ -591,7 +632,7 @@ impl std::fmt::Debug for Notification {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
write!(
f,
"Notification: Selector {} from {:?}",
"Notification: Selector {:?} from {:?}",
self.symbol, self.source
)
}
Expand Down
5 changes: 4 additions & 1 deletion druid/src/win_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,10 @@ impl<T: Data> AppState<T> {
log::warn!("SHOW_WINDOW command must target a window.")
}
_ => {
self.inner.borrow_mut().dispatch_cmd(cmd);
let handled = self.inner.borrow_mut().dispatch_cmd(cmd.clone());
if !handled.is_handled() && cmd.must_be_used() {
log::warn!("{:?} was not handled.", cmd);
}
}
}
}
Expand Down