-
-
Notifications
You must be signed in to change notification settings - Fork 10
Add math styling module #20
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
base: main
Are you sure you want to change the base?
Conversation
A bit more context: #5 (comment). |
Regarding the implementation, as well as having many |
Regardless of my above comments, @laurmaedje will have to approve moving those functions before we merge this PR. |
The issue I had with something like this is that some of the styles will output multiple characters. I also considered implementing the |
Can you not return
Instead, maybe |
Also, you can implement |
Looks like there's quite a bit more going on here than in typst-layout/src/math/text.rs, e.g. the looped and chancery stuff. Would that somehow be exposed in Typst or is it for completeness? |
Regarding EcoString: I wouldn't add a dependency on it here. Doesn't seem worth it to me. |
Yes, I plan to expose these in Typst as well. At this point in time it is just for completeness though. For the Arabic ones I plan to add to Variants the functions As for the chancery/roundhand stuff, we'd add |
224e505
to
9bc35ae
Compare
I've changed the
Since I've removed the trait for now, this no longer applies.
I think in my previous reply to this I misunderstood what you were saying, sorry. I've done that now. Also, I was wondering whether to make the I also need to double check Arabic, as for the normal style the math symbols have separate codepoints (unlike latin where the upright serif for math is the same as the standard codepoints), I think. |
Maybe I did not understand the purpose of |
I agree that we shouldn't have |
When you say not having |
I hadn't seen that those are public. Feels slightly duplicate to me with the enum. It's nice to have have in the type that they return just one char. I guess it depends a bit on how you plan to use them in Typst. But, even if we keep separate functions, I think it would make more sense for the ones that can return multiple chars to return |
ebd2535
to
6038ac3
Compare
Ok I've redone some stuff and more thoroughly documented/commented everything, which makes the code all much clearer I think. I also double checked the deltas and cross referenced with MathML Core. I think this ready from my end to review.
I just decided to make those functions private in the end. |
Oh there is one thing left: we should maybe (?) switch alef, bet, dalet, gimel to map to their mathematical glyphs instead of the Hebrew letters. Also maybe remove shin? Done this in the latest commit, can always undo/modify. |
use std::fmt::{self, Write}; | ||
use std::iter::FusedIterator; | ||
|
||
/// The version of [Unicode](https://www.unicode.org/) that this version of the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// The version of [Unicode](https://www.unicode.org/) that this version of the | |
/// The version of [Unicode](https://www.unicode.org/versions/Unicode16.0.0/core-spec/) that this version of the |
This may be a more useful link.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Though that would then need updating with each version change as well. Not sure if this is really much of a point though. IIRC I basically mirrored what other Unicode crates do, including in the std library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, the tuple below will have to be updated as well anyway. But alternatively you can replace Unicode16.0.0
with latest
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That could also cause issues as latest
might not match the version here 😅
69fb82e
to
80491ac
Compare
What is the status of this exactly? |
I need to open a PR on the Typst side. I think I almost finished it a while ago, I'll get to it soon... |
80491ac
to
0bcf9e9
Compare
0bcf9e9
to
ff20c5e
Compare
Ok, this should all be done for the codex side of things. See typst/typst#6309 for the accompanying Typst PR. The last commit moves the The reason for the |
This PR adds functions and the related infrastructure needed to take a character/string and a style in math (e.g. blackboard-bold, monospace), and return it in styled form. It is implemented under the feature
styling
, which I've added as a default.Moving this to Codex was brought up briefly on Discord, and whilst it doesn't directly fall under "naming Unicode symbols", I think it still fits quite nicely in this crate. I may have jumped the gun a little on this, but it wasn't much work to do. Hopefully this PR starts some more thorough discussion of this idea. I also think that moving this outside of the main typst repo will be an improvement and make it more maintainable.