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

Qvariant improvements #308

Merged
merged 8 commits into from
Feb 25, 2024
Merged

Conversation

direc85
Copy link
Contributor

@direc85 direc85 commented Feb 22, 2024

  • Add to_qstring() wrapper
    • More versatile and efficient than .to_qbytearray().to_string()
    • According to documentation e.g. QVariant(QColor) shouldn't be toString()able but it is
  • Add From<QVariantMap> wrapper
    • Can save a move/clone on Rust side
  • Improve debug print
    • Add type_name wrapper
    • Include the type QVariant contains with printable value, if any
    • Makes debugging QML<>Rust interactions easier

In Whisperfish for example debug prints like this are now possible:

tracing::debug!(tracing::debug!("Attachment: {:?}", attachment);
// Attachment: {data: QVariant(QString: "/home/defaultuser/Downloads/test.md"), type: QVariant(QString: "text/markdown")}

Whisperfish!570 currently depends on this (but it can (barely) do without)

@direc85 direc85 requested a review from rubdos February 23, 2024 22:06
Copy link
Member

@ogoffart ogoffart left a comment

Choose a reason for hiding this comment

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

Thanks, that looks good.

Comment on lines +101 to +102
let data = self.to_qstring().to_string();
let qtype = self.type_name().to_string();
Copy link
Member

Choose a reason for hiding this comment

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

it's probably not needed to convert to a rust String here.

@ogoffart ogoffart merged commit 728220c into woboq:master Feb 25, 2024
19 checks passed
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.

3 participants