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

Add color And \\ulnone #10

Merged
merged 8 commits into from
Apr 7, 2024
Merged

Add color And \\ulnone #10

merged 8 commits into from
Apr 7, 2024

Conversation

zyk-mjzs
Copy link
Contributor

@zyk-mjzs zyk-mjzs commented Apr 4, 2024

Add color property
Increase judgment on \ ulnone for cancel underline

Add parse_color_table and parse_underline on parser.rs

Copy link
Owner

@d0rianb d0rianb left a comment

Choose a reason for hiding this comment

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

Thanks for your commit, the code is very clean.
However, I made a few comments on some details.

src/parser.rs Outdated
ControlWord::Unknown(name) => {
if let Property::Value(key) = property {
match *name {
"\\red" => {
Copy link
Owner

Choose a reason for hiding this comment

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

\red \green and \blue should be Token

src/document.rs Outdated
#[cfg(feature="serde_support")]
use serde::{Deserialize, Serialize};

#[cfg(feature="serde_support")]
Copy link
Owner

Choose a reason for hiding this comment

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

Using #[cfg_attr(feature = "serde_support", derive(Deserialize, Serialize))] would prevent the code duplication

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thx, I got it

@d0rianb d0rianb merged commit aa125af into d0rianb:master Apr 7, 2024
1 check passed
@d0rianb
Copy link
Owner

d0rianb commented Apr 7, 2024

Wonderfull, thanks for all the efforts you put in this PR.

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.

2 participants