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.

Sign up for GitHub

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

Add QStringList::isEmpty missing method #761

Closed
wants to merge 1 commit into from
Closed

Add QStringList::isEmpty missing method #761

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions crates/cxx-qt-lib/src/core/qstringlist.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ mod ffi {
/// Returns a list of all the strings containing the substring str.
fn filter(self: &QStringList, str: &QString, cs: CaseSensitivity) -> QStringList;

/// Returns true if the list has size 0; otherwise returns false.
#[rust_name = "is_empty"]
fn isEmpty(self: &QStringList) -> bool;
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think the reason we didn't have it before is that QStringList is effectively a QList<QString>, so the idea was that you would do QList<QString>::from(&list).is_empty().

As the question is do we want to duplicate the whole QList API again for QStringList here, it's unfortunate we can't inherit easily :-/ Initially we were thinking QStringList would have it's specific API and you get a reference to it casted to a QList if you need any of that API. (Guess maybe we need a mutable reference as well and it should be documented better?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep we will duplicate it :(
If we don't duplicate it we need to add info about "QList::from(&list).is_empty()" for example.
Otherwise we don't know how to do it.
I had the problem when I wanted to implement test for qcommandlineparser

I used is_empty() but it was not defined :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we impl Deref for QStringList to QList then Rust might be able to infer that .is_empty() is actually .as_ref().is_empty() 🤔 Maybe this is the way to go ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree that a Deref implementation would be best 🤔
That should hopefully be rather easy to do 🤞

Copy link
Contributor

Choose a reason for hiding this comment

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

jnbooth is doing this in #1151, so I'm closing this attempt


/// Joins all the string list's strings into a single string with each element
/// separated by the given separator (which can be an empty string).
fn join(self: &QStringList, separator: &QString) -> QString;
Expand Down
Loading