Text API redesign #417
Merged
Text API redesign #417
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rj00a
reviewed
Jul 20, 2023
| } | ||
|
|
||
| impl NormalColor { | ||
| pub const fn as_hex_digit(self) -> char { |
There was a problem hiding this comment.
Suggested change
| pub const fn as_hex_digit(self) -> char { | |
| pub const fn to_hex_digit(self) -> char { |
There was a problem hiding this comment.
Since NormalColor is Copy maybe the best solution here would be simple fn hex_digit(self) and fn name(self) without either as or to to avoid confusion?
There was a problem hiding this comment.
I would associate fn hex_digit(self) with an accessor while fn to_hex_digit(self) with a function that does some work to convert to another type.
qualterz
reviewed
Jul 21, 2023
|
Thanks for the reviews, sorry for my delayed response. Should be good to go now. |
rj00a
approved these changes
Jul 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objective
Solution
Colorto makeresetrepresentable, separateNormalColorandRgbColor.Text,InnerText,TextContent, etc public.TextFormattrait to allow mutating a&mut Textwithout cloning.IntoTexttrait which is basically the same asInto<Cow<'a, Text>>(can't implement the latter because of Rust's trait coherence rules)I couldn't figure out a good way to meaningfully reduce the size of
TextInnerso that it wouldn't need to be boxed inText,