Skip to content

Conversation

@robertbastian
Copy link
Member

icu_properties already does this for unicode-bidi. The current icu_harfbuzz crate is written for use with the harfbuzz crate, however there are other Rust HarfBuzz implementations that don't need the code in that crate.

Copy link
Member

@sffc sffc left a comment

Choose a reason for hiding this comment

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

Seems good to eliminate a leaf crate. Can we avoid growing the boilerplate in examples/cargo/harfbuzz?

@robertbastian
Copy link
Member Author

the boilerplate in the example is additional example code for using buffer providers, as well as the ZST box optimization that not relevant to all HarfBuzz implementations (the harfbuzz crate needs it because it needs to box over FFI).

@robertbastian robertbastian marked this pull request as ready for review October 30, 2025 17:44
@robertbastian robertbastian requested review from sffc and removed request for echeran October 30, 2025 17:44
Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Overall in favor of this!

@hsivonen wrote this, we should probably wait for him to check in

Copy link
Member

@Manishearth Manishearth left a comment

Choose a reason for hiding this comment

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

Would still prefer that @hsivonen sign off on this being removed.

@hsivonen
Copy link
Member

hsivonen commented Nov 3, 2025

Moving the trait implementations seems OK.

It would be good to publish a tombstone release of icu_harfbuzz (with [badges] maintenance = { status = "deprecated" }) to crates.io when when icu_properties and icu_normalizer with this change reach crates.io.

@Manishearth
Copy link
Member

That's a good point. @robertbastian can you do that after merging this?

(my r+ stands, feel free to merge)

@robertbastian
Copy link
Member Author

I filed #7210 because we can only do this after 2.2.

@robertbastian robertbastian merged commit 2aaac23 into unicode-org:main Nov 3, 2025
54 of 57 checks passed
@robertbastian robertbastian deleted the harfbuzz branch November 3, 2025 11:01
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