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

String: Add .is-empty and .length properties #7192

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

task-jp
Copy link
Contributor

@task-jp task-jp commented Dec 21, 2024

Introduce two new methods for string properties in .slint:

  • .is_empty(): Checks if a string is empty.
  • .length(): Retrieves the length of the string.

These additions enhance functionality and improve convenience when working with string properties.

@task-jp task-jp force-pushed the string_length branch 2 times, most recently from 1a94a2f to aea12f3 Compare December 21, 2024 16:50
@ogoffart
Copy link
Member

Thanks a lot for the PR.
Code looks good. One thing missing are the docs.

What is the usecase for this?
I think is-empty make sense (even though it can be done with == "")
But I'm wondering about length, because it is not well defined. Currently it is utf8 bytes. But what can one do with this info? Perhaps one want the number of codepoints? Or does one want to get the size in pixel depending on the font?
Given that, I don't feel so good about adding this function.

(I also wonder if it shouldn't be a property instead of a function, like the one on model/array)

@task-jp
Copy link
Contributor Author

task-jp commented Dec 23, 2024

Thanks for your comment.

Code looks good. One thing missing are the docs.

How can I generate the docs like https://docs.slint.dev/latest/docs/slint/reference/primitive-types/#images ?

I've tried https://github.com/slint-ui/slint/blob/master/docs/development.md#documentation but it seems that only rust documentation is generated.

What is the usecase for this?

For length(), I wanted it to calculate password length in LineEdit.

Perhaps one want the number of codepoints?

Your are right. I will figure out how it would be.

(I also wonder if it shouldn't be a property instead of a function, like the one on model/array)

I thought so, then I found that is-float() was implemented as a function. I will check model/array and try to make them as properties.

Introduce two new properties for string in .slint:
- .is-empty: Checks if a string is empty.
- .length: Retrieves the number of grapheme clusters
  https://www.unicode.org/reports/tr29/#Grapheme_Cluster_Boundaries

These additions enhance functionality and improve convenience when working with string properties.
@task-jp
Copy link
Contributor Author

task-jp commented Dec 24, 2024

I read https://github.com/slint-ui/slint/blob/master/docs/astro/README.md and was able to build the slintdoc.

@task-jp task-jp changed the title String: Add .is_empty() and .length() Methods String: Add .is-empty and .length properties Dec 24, 2024
@hunger
Copy link
Member

hunger commented Jan 6, 2025

@ogoffart: Ping? @task-jp seems to have addressed all the issues you brought up.

@tronical
Copy link
Member

tronical commented Jan 6, 2025

Olivier and I discussed this a bit in a call and came to the following conclusion:

  1. is-empty is a sensible property to have on the string type. It's a good name, it's O(1) to compute. 👍
  2. length on the other hand is tricky for two reasons: As a property as well as by its name this suggests as well that it's O(1) to compute, but it's O(n). The second tricky aspect is that the length of a string is ambiguous. The name itself doesn't tell me what the unit is, and unfortunately for a string there are multiple options, from unicode scalar count, bytes when utf-8 encoded, or count of grapheme clusters. We think that the latter is good choice to have access to in the Slint language. Therefore we propose that instead of length as a property it should be a function that counts the grapheme clusters, so count-grapheme-clusters(). This emphasises that work of counting is to be done, and it makes it clear what's returned from the function.
  3. Implementation wise, we think that your approach of merely exposing this in the Slint language and not in C++/Rust SharedString is good. However the implementation for the Rust generated code assumes an implicit dependency to the unicode-segmentation crate in the application's Cargo.toml, and that's not good as use of this new function would result in compile errors in generated code. Instead, please re-export unicode_segmentation in private_unstable_api::re_exports and use it from there in the generated code.

Thanks for working on this :)

@tronical
Copy link
Member

tronical commented Jan 6, 2025

Subsequence discussion in Mattermost yielded a new compromise on the new name for length. So this amends (2) and we suggest to merely rename length to character-count (as a property, not a function).

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.

4 participants