Skip to content

Commit f76818c

Browse files
committed
refactor: Improve naming
1 parent 88a6c34 commit f76818c

File tree

10 files changed

+98
-68
lines changed

10 files changed

+98
-68
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Changed
6+
7+
- Standardized the formatting of CSS declarations: now consistently using `: ` separator between properties and values.
8+
59
### Performance
610

711
- Various performance improvements.

bindings/python/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Changed
6+
7+
- Standardized the formatting of CSS declarations: now consistently using `: ` separator between properties and values.
8+
59
### Performance
610

711
- Various performance improvements.

bindings/python/tests-py/test_inlining.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ def make_html(style: str, body: str) -> str:
1717
strong { text-decoration:none }
1818
p { font-size:2px }
1919
p.footer { font-size: 1px}"""
20-
SAMPLE_INLINED = """<h1 style="color:red;">Big Text</h1>
21-
<p style="font-size:2px ;"><strong style="text-decoration:none ;">Yes!</strong></p>
20+
SAMPLE_INLINED = """<h1 style="color: red;">Big Text</h1>
21+
<p style="font-size: 2px;"><strong style="text-decoration: none;">Yes!</strong></p>
2222
<p class="footer" style="font-size: 1px;">Foot notes</p>"""
2323

2424

bindings/ruby/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Changed
6+
7+
- Standardized the formatting of CSS declarations: now consistently using `: ` separator between properties and values.
8+
59
### Performance
610

711
- Various performance improvements.

bindings/ruby/spec/css_inline_spec.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def make_html(style, body)
1414
p.footer { font-size: 1px}
1515
"""
1616

17-
SAMPLE_INLINED = "<h1 style=\"color:red;\">Big Text</h1><p style=\"font-size:2px ;\"><strong style=\"text-decoration:none ;\">Yes!</strong></p><p class=\"footer\" style=\"font-size: 1px;\">Foot notes</p>"
17+
SAMPLE_INLINED = "<h1 style=\"color: red;\">Big Text</h1><p style=\"font-size: 2px;\"><strong style=\"text-decoration: none;\">Yes!</strong></p><p class=\"footer\" style=\"font-size: 1px;\">Foot notes</p>"
1818
SAMPLE_HTML = make_html(
1919
SAMPLE_STYLE,
2020
"<h1>Big Text</h1><p><strong>Yes!</strong></p><p class=\"footer\">Foot notes</p>"

bindings/wasm/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## [Unreleased]
44

5+
### Changed
6+
7+
- Standardized the formatting of CSS declarations: now consistently using `: ` separator between properties and values.
8+
59
### Performance
610

711
- Various performance improvements.

bindings/wasm/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ pub mod tests {
147147
.expect("Inlines correctly");
148148
assert_eq!(
149149
result,
150-
"<html><head></head><body><h1 style=\"color:red;\">Test</h1></body></html>"
150+
"<html><head></head><body><h1 style=\"color: red;\">Test</h1></body></html>"
151151
);
152152
}
153153

@@ -163,6 +163,6 @@ pub mod tests {
163163
options,
164164
)
165165
.expect("Inlines correctly");
166-
assert_eq!(result, "<html><head><style>h1 { color:red; }</style></head><body><h1 style=\"color:red;\">Test</h1></body></html>");
166+
assert_eq!(result, "<html><head><style>h1 { color:red; }</style></head><body><h1 style=\"color: red;\">Test</h1></body></html>");
167167
}
168168
}

bindings/wasm/tests-ts/css_inline.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe("CSS inliner", () => {
99
"<html><head><style>h1 { color:red; }</style></head><body><h1>Test</h1></body></html>"
1010
)
1111
).to.equal(
12-
'<html><head></head><body><h1 style="color:red;">Test</h1></body></html>'
12+
'<html><head></head><body><h1 style="color: red;">Test</h1></body></html>'
1313
);
1414
});
1515
it("style tag is kept", function () {
@@ -19,7 +19,7 @@ describe("CSS inliner", () => {
1919
{ keep_style_tags: true }
2020
)
2121
).to.equal(
22-
'<html><head><style>h1 { color:red; }</style></head><body><h1 style="color:red;">Test</h1></body></html>'
22+
'<html><head><style>h1 { color:red; }</style></head><body><h1 style="color: red;">Test</h1></body></html>'
2323
);
2424
});
2525
});

css-inline/src/lib.rs

Lines changed: 71 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -320,24 +320,25 @@ impl<'a> CSSInliner<'a> {
320320
styles.sort_unstable_by(|_, (a, _), _, (b, _)| a.cmp(b));
321321
match element.attributes.get_style_entry() {
322322
Entry::Vacant(entry) => {
323-
let size_estimate: usize = styles
323+
let estimated_declaration_size: usize = styles
324324
.iter()
325325
.map(|(name, (_, value))| {
326-
name.len().saturating_add(value.len()).saturating_add(2)
326+
name.len()
327+
.saturating_add(STYLE_SEPARATOR.len())
328+
.saturating_add(value.len())
329+
// Additional byte for the semicolon symbol
330+
.saturating_add(1)
327331
})
328332
.sum();
329-
let mut final_styles = String::with_capacity(size_estimate);
330-
for (name, (_, value)) in styles {
331-
final_styles.push_str(name);
332-
final_styles.push(':');
333-
replace_double_quotes!(final_styles, name, value);
334-
final_styles.push(';');
333+
let mut buffer = String::with_capacity(estimated_declaration_size);
334+
for (property, (_, value)) in styles {
335+
write_declaration(&mut buffer, property, value);
336+
buffer.push(';');
335337
}
336-
entry.insert(final_styles.into());
338+
entry.insert(buffer.into());
337339
}
338340
Entry::Occupied(mut entry) => {
339-
let existing_style = entry.get_mut();
340-
merge_styles(existing_style, styles, &mut style_buffer)?;
341+
merge_styles(entry.get_mut(), styles, &mut style_buffer)?;
341342
}
342343
}
343344
}
@@ -462,96 +463,109 @@ macro_rules! push_or_update {
462463
}
463464

464465
#[inline]
465-
fn append_style(style: &mut String, name: &str, value: &str) {
466+
fn write_declaration(style: &mut String, name: &str, value: &str) {
466467
style.push_str(name);
467468
style.push_str(STYLE_SEPARATOR);
468469
replace_double_quotes!(style, name, value.trim());
469470
}
470471

471-
/// Merge a new set of styles into an existing one, considering the rules of CSS precedence.
472+
/// Merge a new set of styles into an current one, considering the rules of CSS precedence.
472473
///
473474
/// The merge process maintains the order of specificity and respects the `!important` rule in CSS.
474475
fn merge_styles(
475-
existing_style: &mut StrTendril,
476+
current_style: &mut StrTendril,
476477
new_styles: &IndexMap<&str, (Specificity, &str)>,
477-
style_buffer: &mut SmallVec<[String; 8]>,
478+
declarations_buffer: &mut SmallVec<[String; 8]>,
478479
) -> Result<()> {
479480
// This function is designed with a focus on reusing existing allocations where possible
480-
// We start by parsing the existing declarations in the "style" attribute
481-
let mut input = cssparser::ParserInput::new(existing_style);
482-
let mut parser = cssparser::Parser::new(&mut input);
483-
let declarations =
481+
// We start by parsing the current declarations in the "style" attribute
482+
let mut parser_input = cssparser::ParserInput::new(current_style);
483+
let mut parser = cssparser::Parser::new(&mut parser_input);
484+
let current_declarations =
484485
cssparser::DeclarationListParser::new(&mut parser, parser::CSSDeclarationListParser);
485486
// We manually manage the length of our buffer. The buffer may contain slots used
486487
// in previous runs, and we want to access only the portion that we build in this iteration
487-
// NOTE: Can't use `count`, because `declarations` should be evaluated just once
488-
let mut length: usize = 0;
489-
for (idx, declaration) in declarations.enumerate() {
490-
length = length.saturating_add(1);
491-
let (name, value) = declaration?;
492-
let size_estimate = name
488+
let mut parsed_declarations_count: usize = 0;
489+
for (idx, declaration) in current_declarations.enumerate() {
490+
parsed_declarations_count = parsed_declarations_count.saturating_add(1);
491+
let (property, value) = declaration?;
492+
let estimated_declaration_size = property
493493
.len()
494-
.saturating_add(value.len())
495-
.saturating_add(STYLE_SEPARATOR.len());
496-
// We store the existing style declarations in the `style_buffer` for later merging with new styles
497-
// If possible, we reuse existing slots in the `style_buffer` to avoid additional allocations
498-
if let Some(style) = style_buffer.get_mut(idx) {
499-
style.clear();
500-
style.reserve(size_estimate);
501-
append_style(style, &name, value);
494+
.saturating_add(STYLE_SEPARATOR.len())
495+
.saturating_add(value.len());
496+
// We store the existing style declarations in the buffer for later merging with new styles
497+
// If possible, we reuse existing slots in the buffer to avoid additional allocations
498+
if let Some(buffer) = declarations_buffer.get_mut(idx) {
499+
buffer.clear();
500+
buffer.reserve(estimated_declaration_size);
501+
write_declaration(buffer, &property, value);
502502
} else {
503-
let mut style = String::with_capacity(size_estimate);
504-
append_style(&mut style, &name, value);
505-
style_buffer.push(style);
503+
let mut buffer = String::with_capacity(estimated_declaration_size);
504+
write_declaration(&mut buffer, &property, value);
505+
declarations_buffer.push(buffer);
506506
};
507507
}
508508
// Next, we iterate over the new styles and merge them into our existing set
509509
// New rules will not override old ones unless they are marked as `!important`
510-
for (name, (_, value)) in new_styles {
510+
for (property, (_, value)) in new_styles {
511511
match (
512512
value.strip_suffix("!important"),
513-
style_buffer.iter_mut().find(|style| {
514-
style.starts_with(name)
515-
&& style.get(name.len()..=name.len().saturating_add(1)) == Some(STYLE_SEPARATOR)
513+
declarations_buffer.iter_mut().find(|style| {
514+
style.starts_with(property)
515+
&& style.get(property.len()..=property.len().saturating_add(1))
516+
== Some(STYLE_SEPARATOR)
516517
}),
517518
) {
518519
// The new rule is `!important` and there's an existing rule with the same name
519520
// In this case, we override the existing rule with the new one
520-
(Some(value), Some(style)) => {
521+
(Some(value), Some(buffer)) => {
521522
// We keep the rule name and the colon-space suffix - '<rule>: `
522-
style.truncate(name.len().saturating_add(STYLE_SEPARATOR.len()));
523-
style.push_str(value.trim());
523+
buffer.truncate(property.len().saturating_add(STYLE_SEPARATOR.len()));
524+
buffer.push_str(value.trim());
524525
}
525526
// There's no existing rule with the same name, but the new rule is `!important`
526527
// In this case, we add the new rule with the `!important` suffix removed
527-
(Some(value), None) => push_or_update!(style_buffer, length, name, value),
528+
(Some(value), None) => {
529+
push_or_update!(
530+
declarations_buffer,
531+
parsed_declarations_count,
532+
property,
533+
value
534+
);
535+
}
528536
// There's no existing rule with the same name, and the new rule is not `!important`
529537
// In this case, we just add the new rule as-is
530-
(None, None) => push_or_update!(style_buffer, length, name, value),
538+
(None, None) => push_or_update!(
539+
declarations_buffer,
540+
parsed_declarations_count,
541+
property,
542+
value
543+
),
531544
// Rule exists and the new one is not `!important` - leave the existing rule as-is and
532545
// ignore the new one.
533546
(None, Some(_)) => {}
534547
}
535548
}
536549

537-
// We can now dispose of the parser input, which allows us to clear the `existing_style` string
538-
drop(input);
539-
// Now we prepare to write the merged styles back into `existing_style`
540-
existing_style.clear();
541-
let size_estimate: usize = style_buffer[..length]
550+
// We can now dispose of the parser input, which allows us to clear the current style string
551+
drop(parser_input);
552+
// Now we prepare to write the merged styles back into current style
553+
current_style.clear();
554+
let size_estimate: usize = declarations_buffer[..parsed_declarations_count]
542555
.iter()
556+
// Additional byte for the semicolon symbol
543557
.map(|s| s.len().saturating_add(1))
544558
.sum();
545559

546-
// Reserve enough space in existing_style for the merged style string
547-
existing_style.reserve(u32::try_from(size_estimate).expect("Size overflow"));
560+
// Reserve enough space in current style for the merged style string
561+
current_style.reserve(u32::try_from(size_estimate).expect("Size overflow"));
548562

549-
// Write the merged styles into `existing_style`
550-
for style in &style_buffer[..length] {
551-
if !existing_style.is_empty() {
552-
existing_style.push_char(';');
563+
// Write the merged styles into the current style
564+
for declaration in &declarations_buffer[..parsed_declarations_count] {
565+
if !current_style.is_empty() {
566+
current_style.push_char(';');
553567
}
554-
existing_style.push_slice(style);
568+
current_style.push_slice(declaration);
555569
}
556570
Ok(())
557571
}

css-inline/tests/test_inlining.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,8 @@ p.footer { font-size: 1px}"#,
5050
<p><strong>Yes!</strong></p>
5151
<p class="footer">Foot notes</p>"#,
5252
// Then all styles should be added to new "style" attributes
53-
expected = r#"<h1 style="color:red;">Big Text</h1>
54-
<p style="font-size:2px ;"><strong style="text-decoration:none ;">Yes!</strong></p>
53+
expected = r#"<h1 style="color: red;">Big Text</h1>
54+
<p style="font-size: 2px;"><strong style="text-decoration: none;">Yes!</strong></p>
5555
<p class="footer" style="font-size: 1px;">Foot notes</p>"#
5656
)
5757
}
@@ -242,7 +242,7 @@ fn href_attribute_unchanged() {
242242
243243
</head>
244244
<body>
245-
<h1 style="color:blue;">Big Text</h1>
245+
<h1 style="color: blue;">Big Text</h1>
246246
<a href="https://example.org/test?a=b&amp;c=d">Link</a>
247247
248248
</body></html>"#
@@ -667,7 +667,7 @@ fn inline_to() {
667667
css_inline::inline_to(&html, &mut out).unwrap();
668668
assert_eq!(
669669
String::from_utf8_lossy(&out),
670-
"<html><head></head><body><h1 style=\"color: blue ;\">Big Text</h1></body></html>"
670+
"<html><head></head><body><h1 style=\"color: blue;\">Big Text</h1></body></html>"
671671
)
672672
}
673673

0 commit comments

Comments
 (0)