remove Env type and Data trait #53
Conversation
herkhinah
changed the title
herkhinah
force-pushed
the
shekhinah/remove-env
branch
from
0a84292 to
f40944c
Compare
herkhinah
changed the title
|
Sorry for the late reply, I've been on vacation. I've only had time to skim this, but it looks like a clean refactor. Pretty high-quality contribution! I've noticed a I'll take another look at this soon, and after that, we'll try to get this PR merged ASAP. |
There was a problem hiding this comment.
Overall this seems like a clean set of changes. Once the items I mentioned are addressed, and the rustfmt and clippy errors are fixed, we can merge this ASAP.
By the way, please make sure to do any followup fixes in a separate commit (eg don't use rebase or git commit --amend) so the fixes don't get lost in this PR's huge changeset.
| // TODO remove env | ||
| /* | ||
| // paint the baseline if we're debugging layout | ||
| if env.get(Env::DEBUG_PAINT) && ctx.widget_state.baseline_offset != 0.0 { |
There was a problem hiding this comment.
The refactor needs to address these kinds of cases. I'd suggest adding debug_paint, debug_widget and debug_widget_id bool attributes to GlobalPassCtx and going from there.
| // TODO Env removal | ||
| /* |
| // TODO env removal | ||
| /* |
| // TODO: Env removal | ||
| /* |
| impl PartialEq for RichText { | ||
| fn eq(&self, other: &Self) -> bool { | ||
| Arc::ptr_eq(&self.buffer, &other.buffer) && Arc::ptr_eq(&self.attrs, &other.attrs) && Arc::ptr_eq(&self.links, &other.links) | ||
| } | ||
| } | ||
|
|
||
| impl Eq for RichText {} |
There was a problem hiding this comment.
This is an invalid implementation of Eq and a dubious implementation of PartialEq. Two buffers with different pointer values but identical content shouldn't return false in PartialEq.
As a placeholder solution, I'd suggest adding a function called eg maybe_eq to the TextStorage trait with the same semantic as the now-removed Data::same(). We can think of a better design once this is merged.
|
FYI, I might rebase this PR on my side (or straight-up redo it from scratch) next week, since I'm actively working on Masonry now. |
Fix some clippy lints Update text snapshots Update documentation and examples Update roadmap Add debug properties to PaintCtx Add maybe_eq method to TextStorage as a monkey-patch --- Based on PR #53 by: Shekhinah Memmel <[email protected]>
|
I've done a bunch of changes on top of this PR and merged them. Thanks for the contribution! |
|
Sry for not responding. I’m currently homeless for an unknown amount of time.Sent from my iPhoneOn 26. Mar 2024, at 15:25, Olivier FAURE ***@***.***> wrote:
I've done a bunch of changes on top of this PR and merged them. Thanks for the contribution!
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Shit. Good luck then. |
I've refactored out the Env type and Data trait. The crate compiles and the examples run. Fixes linebender/xilem#373