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

Add QStringList::isEmpty missing method #761

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Montel
Copy link
Contributor

@Montel Montel commented Nov 28, 2023

No description provided.

@@ -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 🤞

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