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
style: Move font-family outside of mako #19366
Conversation
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.
Looks great, just minor nits.
Thanks a lot for knocking out font.mako.rs
!!! \o/
pub fn generic(name: &Atom) -> (structs::FontFamilyType, u8) { | ||
use gecko_bindings::structs::FontFamilyType; | ||
if *name == atom!("serif") { | ||
(FontFamilyType::eFamily_serif, |
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.
nit: Indentation looks weird here, I'd put all either on the same line, or:
(FontFamilyType::eFamily_serif,
structs::kGenericFont_serif)
Though no big preference...
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.
Oh, I didn't notice it. In the same line might be a little tedious?
Please let me make them in the same indentation in different lines.
name: (&*family.mName).into(), | ||
syntax: FamilyNameSyntax::Quoted, | ||
}), | ||
x => panic!("Found unexpected font SingleFontFamily: {:?}", x), |
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.
nit: Found unexpected FontFamilyType
, maybe?
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.
Yes, nice catch! Thanks!
#[cfg(feature = "servo")] | ||
#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq)] | ||
/// A list of SingleFontFamily | ||
pub struct FontFamilyList(Vec<SingleFontFamily>); |
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.
nit: This could be Box<[SingleFontFamily]>
.
02c0d1a
to
a470ebd
Compare
@bors-servo r+ |
π Commit a470ebd has been approved by |
style: Move font-family outside of mako This is a sub-PR of #19015 Besides, this is the last PR for `font.mako.rs`! π r? emilio --- - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [x] These changes fix #19355 - [x] These changes do not require tests <!-- Reviewable:start --> --- This change isβ[<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19366) <!-- Reviewable:end -->
βοΈ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev |
This is a sub-PR of #19015
Besides, this is the last PR for
font.mako.rs
! πr? emilio
./mach build -d
does not report any errors./mach test-tidy
does not report any errorsThis change isβ