Skip to content

Comments

KeyboardShortcut: Use Display for platform string#10833

Open
LeonMatthes wants to merge 1 commit intoslint-ui:masterfrom
LeonMatthes:lm/shortcut-platform-string
Open

KeyboardShortcut: Use Display for platform string#10833
LeonMatthes wants to merge 1 commit intoslint-ui:masterfrom
LeonMatthes:lm/shortcut-platform-string

Conversation

@LeonMatthes
Copy link
Collaborator

Follow-up on #10828

@LeonMatthes LeonMatthes marked this pull request as ready for review February 20, 2026 12:06
@LeonMatthes LeonMatthes requested a review from tronical February 20, 2026 12:06
}
}

impl From<KeyboardShortcut> for SharedString {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to minimise special casing, then I think the existing slint::ToSharedString trait would be a legit replacement. The implementation is identical, but it will require slightly different gymnastics at the use-sites.

What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aha, ToSharedString should already automatically implemented for anything that implements Display, so I should just be able to remove this, nice 👍

@LeonMatthes LeonMatthes force-pushed the lm/shortcut-platform-string branch from 7c8484a to c3b3b83 Compare February 20, 2026 12:35
@LeonMatthes LeonMatthes requested a review from tronical February 20, 2026 12:44
out: &mut SharedString,
) {
*out = shortcut.to_platform_string();
*out = crate::format!("{shortcut}");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps just *out = shortcut.to_shared_string();? :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aye

@LeonMatthes LeonMatthes force-pushed the lm/shortcut-platform-string branch from c3b3b83 to 81c03b8 Compare February 20, 2026 17:37
@LeonMatthes LeonMatthes requested a review from tronical February 20, 2026 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants