From f0b2e51033be403c0c7fc5b9275ee63369adbb76 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 13:28:20 +0000 Subject: [PATCH 1/2] Fix the attribute bugs found with htmx, and the dev server's methods - An attribute given twice (by a bundle and after it, by a function and its call) is written once, with the later value. The browser keeps the first of two, so the bundle's used to win. - `hx-` names are HTML attributes, like `aria-` and `data-` ones: `hx-get /items` is an error that shows `hx-get=/items`, and a bare `hx-` or `data-` word is a flag (`hx-preserve`, `data-open`). - Right after a `key=value`, a name htmlang doesn't know is reported as the rest of that value, which a comma ended (`split-value`), with the value written with `\,`. - A `]` that ends a list inside a `'...'` in a value (`alert(']')`) is reported (`unbalanced-quote`). - A leading argument that reads as text (`@link [hx-get=/x] Open panel`) is taken as the URL with a warning (`text-as-url`). - `htmlang serve` answers GET and HEAD, gives 405 to other methods, and adds its reload script only to pages, not to fragments. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Hca8AvqJAmkTymf66PBCGk --- DESIGN.md | 35 +++-- README.md | 2 +- crates/htmlang-core/src/codegen.rs | 60 +++++---- crates/htmlang-core/src/diagnostic.rs | 12 ++ crates/htmlang-core/src/parser.rs | 131 ++++++++++++++----- crates/htmlang-core/src/syntax.rs | 31 +++++ crates/htmlang-core/src/vocab.rs | 28 +++- src/bin/htmlang_lsp/docs.rs | 10 +- src/serve.rs | 178 ++++++++++++++++++-------- tests/regressions.rs | 166 ++++++++++++++++++++++-- 10 files changed, 521 insertions(+), 132 deletions(-) diff --git a/DESIGN.md b/DESIGN.md index 7f5e76c..2799160 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -72,16 +72,27 @@ An attribute can take one of three forms: (`spacing 20`, `color red`). - `key=value` is an **HTML attribute** (`id=main`, `type=email`, `aria-label=Close`). -- A bare word is a **flag**: a layout attribute such as `center-x`, or a - boolean HTML attribute such as `required`, `disabled`, `hidden` or `open`. +- A bare word is a **flag**: a layout attribute such as `center-x`, a + boolean HTML attribute such as `required`, `disabled`, `hidden` or `open`, + or a `data-` or `hx-` attribute that has no value (`data-open`, + `hx-preserve`). A comma inside `(...)`, `[...]` or `"..."` doesn't split attributes. Anywhere else, `\,` keeps a comma in the value: ``` @el [transition opacity 0.3s\, transform 0.3s, font-family "Open Sans"\, sans-serif] +@el [hx-get=/news, hx-trigger=load\, every 60s] ``` +Right after a `key=value`, a name htmlang doesn't know is most likely the +rest of that value, which a comma ended: `[hx-trigger=load, every 60s]` is +an error (`split-value`) that shows the value written with `\,`. + +An attribute given twice, by a bundle and after it, by a function and its +call, or written twice, takes the later value. An HTML attribute is +written once, with that value. + A variable fills an attribute's value (`padding $gap`, `alt=$title`), never its name or a whole attribute: attributes come from a [bundle](#definitions), written `[$card]`, and a condition chooses whole @@ -154,7 +165,7 @@ values, `@let`, `@page` and `@meta`. | `\@`, `\--` | `@`, `--` | a line of text that starts with `@` or `--` | | `\$` | `$` | a `$` before a name, which would start a variable | | `\{`, `\}` | `{`, `}` | braces that don't start or end an inline element | -| `\[`, `\]` | `[`, `]` | brackets that don't open or close an attribute list | +| `\[`, `\]` | `[`, `]` | brackets that don't open or close an attribute list; a `'...'` in a value (`alert(']')`) doesn't hide one, so a list that seems to end inside one is reported (`unbalanced-quote`) | | `\,` | `,` | a comma in an attribute's value | | `\"` | `"` | a quote that doesn't start or end quoted text | | `\\` | `\` | a backslash before one of these characters | @@ -906,7 +917,10 @@ since it holds only the `@option` lines under it. A value with a space in it is quoted, as such a label: `@optgroup "Citrus fruits"`. The first word is always the argument, so on an element whose attribute is often left out (`@form`, `@video`, `@audio`), text goes on the lines under it: `@form Sign -in` would make `Sign` the form's action. +in` would make `Sign` the form's action. A first word that reads as text, +a capitalized word with nothing of a URL in it, is taken with a warning +(`text-as-url`): `@link [hx-get=/panel] Open panel` makes `Open` the link's +`href`. A relative URL like that one is written `./Open`. The attribute can be written as an attribute instead, and it means the same: `@link [href=/about]` with its text on the lines under it, or @@ -943,12 +957,13 @@ only the file. It isn't shown, so a style on it is an error. HTML attributes are written `key=value`: `id=main`, `class=note`, `href=/about`, `type=email`, `alt=Logo`, `target=_blank`, -`aria-label=Close menu`, `data-id=42`. Any name works, one with a colon -in it included (`xml:lang=en`, `x-on:click=open`): before an `=` a colon is -part of the name, not a prefix. Boolean attributes are -written bare: `required`, `disabled`, `checked`, `hidden`, `open`, `popover`. -An HTML attribute written like a style (`type email`) is an error that -shows the `key=value` form. A style +`aria-label=Close menu`, `data-id=42`, `hx-get=/items`. Any name works, +one with a colon in it included (`xml:lang=en`, `hx-on:click=open()`): +before an `=` a colon is part of the name, not a prefix. Boolean attributes +are written bare: `required`, `disabled`, `checked`, `hidden`, `open`, +`popover`, and so are `data-` and `hx-` attributes that have no value +(`hx-preserve`). An HTML attribute written like a style (`type email`, +`hx-get /items`) is an error that shows the `key=value` form. A style and an HTML attribute can share a name, because the `=` tells them apart: ``` diff --git a/README.md b/README.md index 058611a..4f7bebb 100644 --- a/README.md +++ b/README.md @@ -135,7 +135,7 @@ htmlang build src -o dist # compile a whole site | Command | Purpose | |---|---| | `build [-o out] [--minify] [--strict]` | Compile every `.hl` file under a directory (into `out/` by default), except libraries: files that hold only `@let`s | -| `serve [dir\|file] [-p PORT] [--open]` | Dev server with live reload | +| `serve [dir\|file] [-p PORT] [--open]` | Dev server with live reload. It serves files, so it answers GET and HEAD (other methods get 405), and reloads pages, not fragments | | `watch [dir\|file] [-o out]` | Recompile on change, without a server | | `check [--format json]` | Report diagnostics without writing output | | `lint [--format json]` | Stricter checks (accessibility, nesting) | diff --git a/crates/htmlang-core/src/codegen.rs b/crates/htmlang-core/src/codegen.rs index 543f420..c7fdfcf 100644 --- a/crates/htmlang-core/src/codegen.rs +++ b/crates/htmlang-core/src/codegen.rs @@ -855,15 +855,9 @@ fn generate_full_inner(doc: &Document, dev: bool) -> String { } // `@page`'s HTML attributes go on `` - let mut html_attrs = String::new(); - for attr in &page.html_attrs { - html_attrs.push(' '); - html_attrs.push_str(&attr.key); - if attr.html { - html_attrs.push_str("=\""); - html_attrs.push_str(&html_escape(attr.value.as_deref().unwrap_or(""))); - html_attrs.push('"'); - } + let mut root_attrs = String::new(); + for attr in html_attrs(&page.html_attrs) { + push_html_attr(&mut root_attrs, attr); } let mut body_attrs = String::new(); emit_class_attr(&mut body_attrs, body_class.as_deref(), None); @@ -888,7 +882,7 @@ fn generate_full_inner(doc: &Document, dev: bool) -> String { let reset_css = reset_css(dev, true, &focus_visible_css); format!( - "{nl}{nl}{nl}{nl}\ + "{nl}{nl}{nl}{nl}\ {meta_html}{nl}{nl}\ {nl}{body}{nl}{nl}", ) @@ -1198,27 +1192,43 @@ pub(crate) fn data_uri(path: &str, data: &[u8]) -> String { } /// Emit HTML attributes: `key=value` ones (except `id` / `class`, which -/// are emitted with the generated class) and bare booleans like `required`. +/// are emitted with the generated class) and flags like `required`. fn emit_html_attrs(out: &mut String, attrs: &[Attribute]) { - for attr in attrs { + for attr in html_attrs(attrs) { let key = attr.key.as_str(); - if attr.html && key != "id" && key != "class" { - out.push(' '); - out.push_str(key); - out.push_str("=\""); - // Quoted text has already lost its quotes (see parser.rs) - out.push_str(&html_escape(attr.value.as_deref().unwrap_or(""))); - out.push('"'); - } else if !attr.html - && attr.value.is_none() - && crate::vocab::BOOLEAN_HTML_ATTRS.contains(&key) - { - out.push(' '); - out.push_str(key); + if key != "id" && key != "class" { + push_html_attr(out, attr); } } } +/// The HTML attributes of `attrs`, `key=value` ones and flags, each name +/// once: the later one wins, as a style does. (Of two attributes with one +/// name, the browser keeps the first, so a bundle's attribute or a +/// function's own would win over one written after it.) +fn html_attrs(attrs: &[Attribute]) -> impl Iterator { + let is_html = + |a: &Attribute| a.html || (a.value.is_none() && crate::vocab::is_html_flag(&a.key)); + attrs.iter().enumerate().filter_map(move |(i, attr)| { + let later = attrs[i + 1..] + .iter() + .any(|other| is_html(other) && other.key == attr.key); + (is_html(attr) && !later).then_some(attr) + }) +} + +/// ` key="value"`, or ` key` for a flag. +fn push_html_attr(out: &mut String, attr: &Attribute) { + out.push(' '); + out.push_str(&attr.key); + if attr.html { + out.push_str("=\""); + // Quoted text has already lost its quotes (see parser.rs) + out.push_str(&html_escape(attr.value.as_deref().unwrap_or(""))); + out.push('"'); + } +} + // True if any direct child is `@in-front` or `@behind`. Such children render // as absolutely positioned overlays, so the parent automatically becomes a // positioning context (position:relative + isolation:isolate). diff --git a/crates/htmlang-core/src/diagnostic.rs b/crates/htmlang-core/src/diagnostic.rs index c430652..aa26df6 100644 --- a/crates/htmlang-core/src/diagnostic.rs +++ b/crates/htmlang-core/src/diagnostic.rs @@ -77,6 +77,9 @@ impl Diagnostic { pub mod code { // Syntax: found while reading the file, before anything runs pub const UNCLOSED_BRACKET: &str = "unclosed-bracket"; + /// A `]` that ends an attribute list inside what reads as a `'...'` + /// string in a value (`hx-on:click=alert(']')`). + pub const UNBALANCED_QUOTE: &str = "unbalanced-quote"; pub const UNEXPECTED_BODY: &str = "unexpected-body"; pub const UNEXPECTED_ARGUMENT: &str = "unexpected-argument"; pub const MISSING_ARGUMENT: &str = "missing-argument"; @@ -115,6 +118,12 @@ pub mod code { pub const INVALID_COLOR: &str = "invalid-color"; pub const INVALID_EXPRESSION: &str = "invalid-expression"; pub const INVALID_JSON: &str = "invalid-json"; + /// A name htmlang doesn't know right after a `key=value`: the rest of + /// the value, which a comma ended (`hx-trigger=load, every 2s`). + pub const SPLIT_VALUE: &str = "split-value"; + /// A leading argument taken as a URL that reads as text (`@link + /// [hx-get=/x] Open panel`, whose href is `Open`). + pub const TEXT_AS_URL: &str = "text-as-url"; // Definitions and functions pub const UNUSED_VARIABLE: &str = "unused-variable"; @@ -174,6 +183,7 @@ pub mod code { /// Every code, for tools and tests. pub const ALL: &[&str] = &[ UNCLOSED_BRACKET, + UNBALANCED_QUOTE, UNEXPECTED_BODY, UNEXPECTED_ARGUMENT, MISSING_ARGUMENT, @@ -195,6 +205,8 @@ pub mod code { INVALID_COLOR, INVALID_EXPRESSION, INVALID_JSON, + SPLIT_VALUE, + TEXT_AS_URL, UNUSED_VARIABLE, UNUSED_BUNDLE, UNUSED_FUNCTION, diff --git a/crates/htmlang-core/src/parser.rs b/crates/htmlang-core/src/parser.rs index 3a69ea2..e01d345 100644 --- a/crates/htmlang-core/src/parser.rs +++ b/crates/htmlang-core/src/parser.rs @@ -2618,8 +2618,7 @@ fn read_page( // one is written let mut seen: HashMap = HashMap::new(); for attr in read { - let boolean = attr.value.is_none() - && crate::vocab::BOOLEAN_HTML_ATTRS.contains(&attr.key.as_str()); + let boolean = attr.value.is_none() && crate::vocab::is_html_flag(&attr.key); // Where it is written, when it is written in the list itself let nth = seen.entry(attr.key.clone()).or_default(); let column = list @@ -3656,6 +3655,9 @@ fn parse_single_element( } } else { let column = Some(token.span.column); + if let Some(d) = text_as_url(&kind, &token, ctx) { + ctx.push_once(d); + } argument = Some( ctx.fill_value(&token.raw, text.span.line, column, Sink::Text) .0, @@ -3813,6 +3815,35 @@ fn bad_leading_token( Some(at_attribute(d, Some(token.span.column), ctx)) } +/// A first word taken as a URL that reads as text: a word with a capital +/// and nothing a URL or a file name has, as in `@link [hx-get=/panel] Open +/// panel`, whose href would be `Open`. It still is (a relative URL can be +/// such a word), with a warning. +fn text_as_url(kind: &ElementKind, token: &syntax::Arg, ctx: &ParseContext) -> Option { + const URLS: &[&str] = &["href", "src", "srcset", "action", "data"]; + let word = token.raw.as_str(); + let reads_as_text = word.starts_with(char::is_uppercase) + && !word.contains([ + '/', '.', ':', '#', '?', '=', '&', '%', '~', '$', '{', '\\', '"', + ]); + if !reads_as_text || !kind.arg().attributes().iter().any(|a| URLS.contains(a)) { + return None; + } + let name = kind.name(); + let attr = leading_name(kind); + let text = if takes_no_text(kind) { + String::new() + } else { + format!(": write the {attr} before the text, or put the text on the lines under it") + }; + let message = format!( + "@{name} takes '{word}', its first word, as its {attr}{text}. A relative URL that \ + starts with a capital is written `./{word}`" + ); + let d = Diagnostic::warning(code::TEXT_AS_URL, token.span.line, message).subject(word); + Some(at_attribute(d, Some(token.span.column), ctx)) +} + /// `@link /a > @image logo.png`: a `>` chains only after an element's /// name or its `[attributes]`, so after a leading argument it is text. /// Said when what follows names an element or a function. @@ -4881,7 +4912,7 @@ fn check_attribute( attr: &Attribute, line: usize, column: Option, - after_another: bool, + before: Option<&Attribute>, ctx: &mut ParseContext, ) -> bool { use crate::vocab; @@ -4909,12 +4940,6 @@ fn check_attribute( &attr.key, ); } - let is_html_name = |name: &str| { - vocab::HTML_ATTRIBUTES.contains(&name) - || name.starts_with("aria-") - || name.starts_with("data-") - }; - let Some(value) = &attr.value else { // `inline` puts a file into the page as it compiles, which no // state or media condition can undo @@ -4933,7 +4958,7 @@ fn check_attribute( if vocab::HTMLANG_FLAGS.contains(&base) { return true; } - if vocab::BOOLEAN_HTML_ATTRS.contains(&base) { + if vocab::is_html_flag(base) { if !prefixed { return true; } @@ -4958,7 +4983,7 @@ fn check_attribute( &attr.key, ); } - if is_html_name(base) && !prefixed { + if vocab::is_html_attribute(base) && !prefixed { return fail( ctx, code::HTML_ATTRIBUTE_FORM, @@ -4969,7 +4994,7 @@ fn check_attribute( base, ); } - return unknown_attribute(attr, base, line, column, after_another, true, ctx); + return unknown_attribute(attr, base, line, column, before, true, ctx); }; if vocab::HTMLANG_FLAGS.contains(&base) { @@ -4989,7 +5014,7 @@ fn check_attribute( { return check_css_value(attr, base, line, column, ctx); } - if is_html_name(base) && !prefixed { + if vocab::is_html_attribute(base) && !prefixed { let diagnostic = Diagnostic::error( code::HTML_ATTRIBUTE_FORM, line, @@ -5004,7 +5029,7 @@ fn check_attribute( return false; } let passes = vocab::is_property_name(base); - unknown_attribute(attr, base, line, column, after_another, !passes, ctx) + unknown_attribute(attr, base, line, column, before, !passes, ctx) && check_css_value(attr, base, line, column, ctx) } @@ -5015,8 +5040,8 @@ fn check_attribute( fn could_be_parameter(attr: &Attribute) -> bool { use crate::vocab; let key = attr.key.as_str(); - let flag = attr.value.is_none() - && (vocab::HTMLANG_FLAGS.contains(&key) || vocab::BOOLEAN_HTML_ATTRS.contains(&key)); + let flag = + attr.value.is_none() && (vocab::HTMLANG_FLAGS.contains(&key) || vocab::is_html_flag(key)); !attr.html && !flag && !vocab::is_style_attribute(key) @@ -5071,7 +5096,7 @@ fn unknown_attribute( base: &str, line: usize, column: Option, - after_another: bool, + before: Option<&Attribute>, error: bool, ctx: &mut ParseContext, ) -> bool { @@ -5117,6 +5142,36 @@ fn unknown_attribute( // itself let suggestion = suggest_closest(base, &crate::vocab::all_attributes()).filter(|&closest| closest != base); + // `hx-trigger=load, every 2s`: right after a `key=value`, a name that + // isn't close to one htmlang knows is the rest of that value, which + // the comma ended. It is left out. + let cut = before.filter(|b| b.html && b.value.is_some()); + let rest = match &attr.value { + Some(value) => format!("{} {}", attr.key, value), + None => attr.key.clone(), + }; + let joined = cut.map(|cut| { + let value = cut.value.as_deref().unwrap_or(""); + ( + format!("{}={}", cut.key, value), + format!("{}={}\\, {}", cut.key, value, rest), + ) + }); + if let (Some((cut, joined)), None) = (&joined, suggestion) { + let diagnostic = Diagnostic::error( + code::SPLIT_VALUE, + line, + format!( + "unknown attribute '{}' after `{}`: if '{}' is the rest of its value, the comma \ + ended it. To keep a comma in a value, write `\\,`: `{}`", + base, cut, rest, joined + ), + ) + .subject(base); + let diagnostic = at_attribute(diagnostic, column, ctx); + ctx.diagnostics.push(diagnostic); + return false; + } let mut message = match (&suggestion, error) { (Some(closest), true) => { format!( @@ -5138,12 +5193,19 @@ fn unknown_attribute( // sans-serif`: the comma ended the attribute, and the rest of the // value became one let rest_of_a_value = !base.starts_with(|c: char| c.is_ascii_alphabetic() || c == '-') - || (suggestion.is_none() && attr.value.is_none() && after_another); - if rest_of_a_value && error { + || (suggestion.is_none() && attr.value.is_none() && before.is_some()); + if joined.is_some() || (rest_of_a_value && error) { if !message.ends_with('?') { message.push('.'); } - message.push_str(" A comma separates attributes: to keep one in a value, write `\\,`"); + match &joined { + Some((cut, joined)) => message.push_str(&format!( + " If it is the rest of `{}`, the comma ended it: write `{}`", + cut, joined + )), + None => message + .push_str(" A comma separates attributes: to keep one in a value, write `\\,`"), + } } let severity = if error { Severity::Error @@ -5285,7 +5347,13 @@ fn read_attrs( && !text_keys.contains(&attr.key); if checked_here && (misspelled_parameter(attr, text_keys, line, column(0), ctx) - || !check_attribute(attr, line, column(0), !attrs.is_empty(), ctx)) + || !check_attribute( + attr, + line, + column(0), + attrs.last().map(|(a, _)| a), + ctx, + )) { continue; } @@ -5389,11 +5457,11 @@ fn read_attrs( if validate { let seen = (normal_key(&attr.key), attr.html); if out.seen_keys.contains(&seen) { - let message = if attr.html { - format!("duplicate attribute '{}='", attr.key) - } else { - format!("duplicate attribute '{}': the later one wins", attr.key) - }; + let written = if attr.html { "=" } else { "" }; + let message = format!( + "duplicate attribute '{}{}': the later one wins", + attr.key, written + ); let diagnostic = Diagnostic::warning(code::DUPLICATE_ATTRIBUTE, line, message) .subject(attr.key.as_str()); let diagnostic = at_attribute(diagnostic, column(0), ctx); @@ -5438,8 +5506,8 @@ fn read_attrs( // A style or a flag: its name is checked, and what can't be // written into the page is an error and left out let checked = validate && !attr.html && !(ctx.in_bundle && could_be_parameter(&attr)); - let after_another = !out.attrs.is_empty(); - if checked && !check_attribute(&attr, line, column(0), after_another, ctx) { + let before = out.attrs.last().map(|(a, _)| a); + if checked && !check_attribute(&attr, line, column(0), before, ctx) { continue; } @@ -5583,7 +5651,7 @@ fn prefixed_member( }; if validate { let fine = if attr.value.is_none() || could_be_parameter(attr) { - check_attribute(&prefixed, line, column, false, ctx) + check_attribute(&prefixed, line, column, None, ctx) } else { check_prefixes(&prefixed, line, column, ctx) }; @@ -5994,6 +6062,7 @@ fn check_branch( attrs: branch.attrs().to_vec(), span: token.span, closed: true, + quoted_close: None, }; let grouped = syntax::Attr { key: prefix.to_string(), @@ -6478,7 +6547,7 @@ fn dropped_by_the_element(elem: &Element, diagnostics: &mut Vec) { let styles: Vec<&Attribute> = elem .attrs .iter() - .filter(|a| !a.html && !crate::vocab::BOOLEAN_HTML_ATTRS.contains(&a.key.as_str())) + .filter(|a| !a.html && !crate::vocab::is_html_flag(&a.key)) .collect(); if !styles.is_empty() { error( @@ -6740,7 +6809,7 @@ fn inline_svg(node: Node, line_num: usize, ctx: &mut ParseContext) -> Node { continue; } (key, true, Some(_)) => key, - (key, false, None) if crate::vocab::BOOLEAN_HTML_ATTRS.contains(&key) => key, + (key, false, None) if crate::vocab::is_html_flag(key) => key, _ => { left_out.push(attr.key.clone()); continue; diff --git a/crates/htmlang-core/src/syntax.rs b/crates/htmlang-core/src/syntax.rs index 252ac02..a85e786 100644 --- a/crates/htmlang-core/src/syntax.rs +++ b/crates/htmlang-core/src/syntax.rs @@ -136,6 +136,10 @@ pub struct AttrList { pub span: Span, /// Whether the closing `]` was found. pub closed: bool, + /// The closing `]`, when it looks like it was meant to be inside a + /// `'...'` string: a `'` before it in the list isn't closed, and the + /// word right after it has one, as in `hx-on:click=alert(']')`. + pub quoted_close: Option, } /// One attribute of a list, as written. @@ -1506,6 +1510,25 @@ impl Reader<'_> { let (text, open) = self.text(0, self.text.len()); (NodeKind::Text(text), open) }; + if let NodeKind::Element(element) = &kind { + let closes = element + .chain + .iter() + .filter_map(|head| head.attrs.as_ref()?.quoted_close); + for close in closes { + problems.push( + Diagnostic::warning( + code::UNBALANCED_QUOTE, + close.line, + "this `]` ends the attribute list, but it looks like it is inside a \ + `'...'` in the value: write `\\]` to keep it in the value" + .to_string(), + ) + .column(close.column) + .source(self.text.to_string()), + ); + } + } if open { // An unclosed quote keeps the list open to the end of the file let mut quotes = 0; @@ -1590,6 +1613,8 @@ impl Reader<'_> { fn attr_list(&self, open: usize, limit: usize) -> (AttrList, usize) { let mut depth = 0; let mut quoted = false; + // The `'`s outside `"..."`, which don't quote anything in htmlang + let mut apostrophes = 0; let mut close = None; let mut i = open; while i < limit { @@ -1603,6 +1628,7 @@ impl Reader<'_> { match c { '"' => quoted = !quoted, _ if quoted => {} + '\'' => apostrophes += 1, '[' => depth += 1, ']' => { depth -= 1; @@ -1622,10 +1648,15 @@ impl Reader<'_> { .filter_map(|(s, e)| self.attr(s, e)) .collect(); let end = close.map_or(limit, |c| c + 1); + let quoted_close = close.filter(|&c| { + let word = self.text[c + 1..limit].split(char::is_whitespace).next(); + apostrophes % 2 == 1 && word.is_some_and(|word| word.contains('\'')) + }); let list = AttrList { attrs, span: self.span(open, end), closed: close.is_some(), + quoted_close: quoted_close.map(|c| self.span(c, c + 1)), }; (list, end) } diff --git a/crates/htmlang-core/src/vocab.rs b/crates/htmlang-core/src/vocab.rs index 03e3742..1a1c663 100644 --- a/crates/htmlang-core/src/vocab.rs +++ b/crates/htmlang-core/src/vocab.rs @@ -5,7 +5,8 @@ //! property (`spacing`, `center-x`, `width fill`, ...), handled in codegen; //! - a standard CSS property, copied into the generated CSS unchanged; //! - an HTML attribute, written `key=value` (or bare, for booleans such as -//! `required`) and emitted on the element. +//! `required` and for `data-` and `hx-` attributes) and emitted on the +//! element. /// htmlang's own attributes (those that are neither a CSS property nor an /// HTML attribute). @@ -158,6 +159,31 @@ pub const BOOLEAN_HTML_ATTRS: &[&str] = &[ "readonly", "required", "reversed", "sandbox", "selected", ]; +/// The families of HTML attributes whose names are open: ARIA's, the +/// author's own `data-` attributes, and htmx's `hx-` attributes. +pub const HTML_ATTRIBUTE_FAMILIES: &[&str] = &["aria-", "data-", "hx-"]; + +/// An HTML attribute by its name: a common one, or one of a family whose +/// names are open (`aria-label`, `data-id`, `hx-get`). +pub fn is_html_attribute(name: &str) -> bool { + HTML_ATTRIBUTES.contains(&name) + || HTML_ATTRIBUTE_FAMILIES + .iter() + .any(|family| in_family(name, family)) +} + +/// An HTML attribute written bare, as a flag, and rendered without a value: +/// a boolean one (`required`), or a `data-` or `hx-` attribute, whose +/// presence is what it says (`data-open`, htmx's `hx-preserve`). ARIA's +/// attributes always take a value. +pub fn is_html_flag(name: &str) -> bool { + BOOLEAN_HTML_ATTRS.contains(&name) || in_family(name, "data-") || in_family(name, "hx-") +} + +fn in_family(name: &str, family: &str) -> bool { + name.len() > family.len() && name.starts_with(family) +} + /// Common HTML attribute names, used to suggest `key=value` when one is /// written like a style (`type email`). Any name works with `=`. #[rustfmt::skip] diff --git a/src/bin/htmlang_lsp/docs.rs b/src/bin/htmlang_lsp/docs.rs index d095389..3f185a1 100644 --- a/src/bin/htmlang_lsp/docs.rs +++ b/src/bin/htmlang_lsp/docs.rs @@ -399,10 +399,12 @@ fn attribute_hover(name: &str) -> Option { "**{name}** \u{2014} Boolean HTML attribute, written bare: `[{name}]`." )); } - if vocab::HTML_ATTRIBUTES.contains(&name) - || name.starts_with("aria-") - || name.starts_with("data-") - { + if vocab::is_html_flag(name) { + return Some(format!( + "**{name}** \u{2014} HTML attribute, written `{name}=value`, or bare for one with no value: `[{name}]`." + )); + } + if vocab::is_html_attribute(name) { return Some(format!( "**{name}** \u{2014} HTML attribute, written `{name}=value`." )); diff --git a/src/serve.rs b/src/serve.rs index df76faf..ec46c67 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -64,11 +64,10 @@ where } let request = String::from_utf8_lossy(&buf[..n]); - let path = request - .lines() - .next() - .and_then(|line| line.split_whitespace().nth(1)) - .unwrap_or("/"); + let (method, path) = request_line(&request); + let Some(head) = reads(method) else { + return method_not_allowed(&mut stream).await; + }; if path == "/_events" { return handle_sse(stream, reload_rx).await; @@ -91,13 +90,8 @@ where } else { // Generate directory listing let listing = generate_directory_listing(&root_dir); - let header = format!( - "HTTP/1.1 200 OK\r\nContent-Type: text/html; charset=utf-8\r\nContent-Length: {}\r\nCache-Control: no-cache\r\n\r\n", - listing.len(), - ); - stream.write_all(header.as_bytes()).await?; - stream.write_all(listing.as_bytes()).await?; - return Ok(()); + let html = "text/html; charset=utf-8"; + return respond(&mut stream, "200 OK", html, listing.as_bytes(), head).await; } } else { // The file itself, then `.html`, then `/index.html`. @@ -130,23 +124,67 @@ where return Ok(()); } }; - let content_type = content_type_for(&fp); - let header = format!( - "HTTP/1.1 200 OK\r\nContent-Type: {}\r\nContent-Length: {}\r\nCache-Control: no-cache\r\n\r\n", - content_type, - body.len(), - ); - stream.write_all(header.as_bytes()).await?; - stream.write_all(&body).await?; + respond(&mut stream, "200 OK", content_type_for(&fp), &body, head).await?; } None => { - send_404(&mut stream, clean_path).await?; + send_404(&mut stream, clean_path, head).await?; } } Ok(()) } +/// The method and the target of a request, from its first line. +fn request_line(request: &str) -> (&str, &str) { + let mut words = request.lines().next().unwrap_or("").split_whitespace(); + (words.next().unwrap_or("GET"), words.next().unwrap_or("/")) +} + +/// Whether the server reads files for `method`: `Some(true)` for HEAD (the +/// headers only), `Some(false)` for GET, and `None` for any other method, +/// which a server of files can't answer: a form that posts to a page, or +/// htmx's `hx-post`, gets 405 rather than the page. +fn reads(method: &str) -> Option { + match method { + "GET" => Some(false), + "HEAD" => Some(true), + _ => None, + } +} + +async fn method_not_allowed(stream: &mut S) -> std::io::Result<()> +where + S: AsyncRead + AsyncWrite + Unpin + Send, +{ + stream + .write_all( + b"HTTP/1.1 405 Method Not Allowed\r\nAllow: GET, HEAD\r\nContent-Length: 0\r\n\r\n", + ) + .await +} + +/// A response with `body`, which a HEAD request doesn't get. +async fn respond( + stream: &mut S, + status: &str, + content_type: &str, + body: &[u8], + head: bool, +) -> std::io::Result<()> +where + S: AsyncRead + AsyncWrite + Unpin + Send, +{ + let header = format!( + "HTTP/1.1 {status}\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nCache-Control: no-cache\r\n\r\n", + body.len(), + ); + stream.write_all(header.as_bytes()).await?; + if !head { + stream.write_all(body).await?; + } + Ok(()) +} + /// Generate a directory listing HTML page showing available .html pages. fn generate_directory_listing(dir: &Path) -> String { let mut pages = Vec::new(); @@ -214,16 +252,15 @@ where } let request = String::from_utf8_lossy(&buf[..n]); - let path = request - .lines() - .next() - .and_then(|line| line.split_whitespace().nth(1)) - .unwrap_or("/"); + let (method, path) = request_line(&request); + let Some(head) = reads(method) else { + return method_not_allowed(&mut stream).await; + }; if path == "/_events" { handle_sse(stream, reload_rx).await } else { - handle_file(stream, &html_path, root_dir, path).await + handle_file(stream, &html_path, root_dir, path, head).await } } @@ -281,6 +318,7 @@ async fn handle_file( html_path: &Path, root_dir: Option<&Path>, request_path: &str, + head: bool, ) -> std::io::Result<()> where S: AsyncRead + AsyncWrite + Unpin + Send, @@ -302,7 +340,7 @@ where } else if is_within(dir, &requested) { requested } else { - send_404(&mut stream, request_path).await?; + send_404(&mut stream, request_path, head).await?; return Ok(()); }; @@ -315,20 +353,13 @@ where } } Err(_) => { - send_404(&mut stream, request_path).await?; + send_404(&mut stream, request_path, head).await?; return Ok(()); } }; let content_type = content_type_for(&file_path); - let header = format!( - "HTTP/1.1 200 OK\r\nContent-Type: {content_type}\r\nContent-Length: {}\r\nCache-Control: no-cache\r\n\r\n", - body.len(), - ); - stream.write_all(header.as_bytes()).await?; - stream.write_all(&body).await?; - - Ok(()) + respond(&mut stream, "200 OK", content_type, &body, head).await } /// Map a request target (`/a/b.css?v=2`) to a path under `root`. Strips the @@ -373,7 +404,7 @@ fn percent_decode(s: &str) -> Option { String::from_utf8(out).ok() } -async fn send_404(stream: &mut S, path: &str) -> std::io::Result<()> +async fn send_404(stream: &mut S, path: &str, head: bool) -> std::io::Result<()> where S: AsyncRead + AsyncWrite + Unpin + Send, { @@ -390,15 +421,13 @@ where Back to index", path.replace('<', "<").replace('>', ">") ); - let header = format!( - "HTTP/1.1 404 Not Found\r\nContent-Type: text/html; charset=utf-8\r\nContent-Length: {}\r\nCache-Control: no-cache\r\n\r\n", - body.len() - ); - stream.write_all(header.as_bytes()).await?; - stream.write_all(body.as_bytes()).await?; - Ok(()) + let html = "text/html; charset=utf-8"; + respond(stream, "404 Not Found", html, body.as_bytes(), head).await } +/// The page with the reload script before its ``. A fragment +/// (`--partial`) has no ``, and is served as it is: it goes into +/// a page that has the script, and a second one would reload it twice. fn inject_reload_script(content: &[u8]) -> Vec { let html = String::from_utf8_lossy(content); match html.rfind("") { @@ -410,11 +439,7 @@ fn inject_reload_script(content: &[u8]) -> Vec { result.push_str(&html[pos..]); result.into_bytes() } - None => { - let mut result = content.to_vec(); - result.extend_from_slice(RELOAD_SCRIPT.as_bytes()); - result - } + None => content.to_vec(), } } @@ -477,4 +502,57 @@ mod tests { assert_eq!(resolve_request_path(root, "/bad%zz"), None); assert_eq!(resolve_request_path(root, "é"), Some(root.join("é"))); } + + /// The response of the directory server at `root` to `request`. + async fn respond_to(root: &Path, request: &str) -> String { + let (mut client, server) = tokio::io::duplex(1 << 16); + let (reload, _) = broadcast::channel(1); + let server = tokio::spawn(handle_dir_request( + server, + root.to_path_buf(), + reload.subscribe(), + )); + client.write_all(request.as_bytes()).await.unwrap(); + let mut response = Vec::new(); + client.read_to_end(&mut response).await.unwrap(); + server.await.unwrap().unwrap(); + String::from_utf8(response).unwrap() + } + + #[tokio::test] + async fn files_are_read_by_get_and_head_only() { + let root = std::env::temp_dir().join(format!("htmlang-serve-{}", std::process::id())); + std::fs::create_dir_all(&root).unwrap(); + std::fs::write( + root.join("page.html"), + "

Hi

", + ) + .unwrap(); + std::fs::write(root.join("frag.html"), "

Hi

").unwrap(); + + let page = respond_to(&root, "GET /page HTTP/1.1\r\n\r\n").await; + assert!(page.starts_with("HTTP/1.1 200 OK"), "{page}"); + assert!(page.contains(RELOAD_SCRIPT), "{page}"); + + // A fragment goes into a page that already reloads + let fragment = respond_to(&root, "GET /frag.html HTTP/1.1\r\n\r\n").await; + assert!( + fragment.ends_with("\r\n\r\n

Hi

"), + "{fragment}" + ); + + let head = respond_to(&root, "HEAD /page HTTP/1.1\r\n\r\n").await; + assert!(head.starts_with("HTTP/1.1 200 OK"), "{head}"); + assert!(head.ends_with("\r\n\r\n"), "{head}"); + + for method in ["POST", "PUT", "DELETE"] { + let request = format!("{method} /page HTTP/1.1\r\nContent-Length: 3\r\n\r\nq=1"); + let response = respond_to(&root, &request).await; + assert!( + response.starts_with("HTTP/1.1 405 Method Not Allowed\r\nAllow: GET, HEAD"), + "{response}" + ); + } + let _ = std::fs::remove_dir_all(&root); + } } diff --git a/tests/regressions.rs b/tests/regressions.rs index 9d72c0f..91993f3 100644 --- a/tests/regressions.rs +++ b/tests/regressions.rs @@ -546,16 +546,28 @@ fn css_values_pass_through() { #[test] fn page_attributes_are_checked_like_any_element_s() { // `canonical` belongs in a `@head` link: on @page it is an unknown CSS - // property (for ), reported rather than dropped - let result = parser::parse("@page [lang=en, canonical https://x.dev] T\n"); - assert!( - result - .diagnostics - .iter() - .any(|d| d.code == "unknown-attribute" && d.message.contains("'canonical'")), - "{:?}", - result.diagnostics - ); + // property (for ), reported rather than dropped. Right after an + // HTML attribute, it may be the rest of that one's value too. + for (source, code) in [ + ( + "@page [canonical https://x.dev, lang=en] T\n", + "unknown-attribute", + ), + ( + "@page [lang=en, canonical https://x.dev] T\n", + "split-value", + ), + ] { + let result = parser::parse(source); + assert!( + result + .diagnostics + .iter() + .any(|d| d.code == code && d.message.contains("'canonical'")), + "{:?}", + result.diagnostics + ); + } } // --- One grammar: the syntax tree and the checks built on it --- @@ -2322,3 +2334,137 @@ fn a_leading_argument_that_can_t_be_one_is_reported() { .collect(); assert_eq!(lines, [1, 2], "{:?}", d); } + +// --- Found while looking at htmx's attributes --- + +#[test] +fn an_attribute_after_a_bundle_or_a_function_s_own_wins() { + // The browser keeps the first of two attributes with one name, so + // writing both made the bundle's win over the one written after it + let out = compile("@let live [hx-get=/x, hx-trigger=load]\n@el [$live, hx-trigger=click] A\n"); + assert!( + out.contains(r#"hx-get="/x" hx-trigger="click">"#), + "{}", + out + ); + assert_eq!(out.matches("hx-trigger=").count(), 1, "{}", out); + let out = compile( + "@let @action [url]\n @button [type=button, hx-post=$url, hx-target=#result] Save\n@action [url /save, hx-target=#other]\n", + ); + assert!(out.contains(r##"hx-target="#other""##), "{}", out); + assert_eq!(out.matches("hx-target=").count(), 1, "{}", out); + let out = compile("@page [lang=en, lang=fr] T\n@text a\n"); + assert!(out.contains(r#""#), "{}", out); + let d = diagnostics_of("@el [hx-get=/a, hx-get=/b] B\n"); + assert!( + d.iter() + .any(|d| d.code == "duplicate-attribute" && d.message.contains("later one wins")), + "{:?}", + d + ); +} + +#[test] +fn a_comma_that_ends_an_html_attribute_s_value_is_reported() { + // The rest of `hx-trigger=load, every 2s` was a CSS property `every`, + // with a warning that didn't say why + let d = diagnostics_of("@el [hx-get=/poll, hx-trigger=load, every 2s] A\n"); + let split = d + .iter() + .find(|d| d.code == "split-value") + .expect("split-value"); + assert_eq!(split.severity, Severity::Error); + assert!( + split.message.contains(r"`hx-trigger=load\, every 2s`"), + "{}", + split.message + ); + // A bare word, as in `hx-confirm=Sure, really?` + assert!(coded_in( + &diagnostics_of("@el [hx-confirm=Sure, really?] A\n"), + "split-value" + )); + // Close to a known name, it is that name misspelled, with a note + let d = diagnostics_of("@el [id=x, colr red] A\n"); + let typo = d + .iter() + .find(|d| d.code == "unknown-attribute") + .expect("unknown-attribute"); + assert!(typo.message.contains("'color'") && typo.message.contains(r"`id=x\, colr red`")); + // An escaped comma, or a known attribute after the comma, is fine + let out = compile("@el [hx-trigger=load\\, every 2s, color red] A\n"); + assert!(out.contains(r#"hx-trigger="load, every 2s""#), "{}", out); +} + +#[test] +fn hx_attributes_are_html_attributes() { + // `hx-get /items` was a CSS property, with a warning + let d = diagnostics_of("@button [hx-get /items] B\n"); + let form = d + .iter() + .find(|d| d.code == "html-attribute-form") + .expect("form"); + assert_eq!(form.severity, Severity::Error); + assert!(form.message.contains("`hx-get=/items`"), "{}", form.message); + // htmx's flags are written bare, like `data-` ones; ARIA's take a value + let out = compile("@el [hx-preserve, data-open, id=player] A\n"); + assert!( + out.contains(r#"id="player" hx-preserve data-open>"#), + "{}", + out + ); + assert!(coded_in( + &diagnostics_of("@el [aria-hidden] A\n"), + "html-attribute-form" + )); + assert!(coded_in( + &diagnostics_of("@el [md:hx-preserve] A\n"), + "invalid-prefix" + )); +} + +#[test] +fn link_text_taken_as_the_url_is_reported() { + // `Open` became the href, and `panel` the text, without a word + let d = diagnostics_of("@link [hx-get=/panel, hx-target=#main] Open panel\n"); + let d = d + .iter() + .find(|d| d.code == "text-as-url") + .expect("text-as-url"); + assert_eq!(d.severity, Severity::Warning); + assert!( + d.message.contains("'Open'") && d.message.contains("href"), + "{}", + d.message + ); + assert!(coded_in(&diagnostics_of("@form Sign in\n"), "text-as-url")); + for fine in [ + "@link /about About\n", + "@link about About us\n", + "@link ./Home Home\n", + "@link https://example.com Out\n", + "@link $url More\n", + "@image [alt=Logo] Logo.svg\n", + "@optgroup Citrus\n @option Lemon\n", + ] { + assert!(!coded_in(&diagnostics_of(fine), "text-as-url"), "{}", fine); + } +} + +#[test] +fn a_bracket_inside_single_quotes_is_reported() { + // The `]` in `alert(']')` ended the list, and the rest became text + let d = diagnostics_of("@button [type=button, hx-on:click=alert(']')] Go\n"); + let d = d + .iter() + .find(|d| d.code == "unbalanced-quote") + .expect("unbalanced-quote"); + assert_eq!(d.column, Some(41)); + // Escaped, it is in the value; an apostrophe in text is fine + let out = compile("@button [type=button, hx-on:click=alert('\\]')] Go\n"); + assert!(out.contains(r#"hx-on:click="alert(']')">Go"#), "{}", out); + assert!(!coded_in( + &diagnostics_of("@el [title=Don't] It's fine\n"), + "unbalanced-quote" + )); +} From 539d6225df596eac8a1f44c90af7ba84ca1908c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 27 Sep 2026 13:39:47 +0000 Subject: [PATCH 2/2] Name generated classes by a hash of their styles A class was named by its place in the file (`hl-a`, `hl-b`, ...), so a fragment's `hl-a` and a page's `hl-a` were two different styles, and a fragment swapped into a page (htmx, or any server-rendered partial) restyled the page's elements and lost its own layout. - A class is `hl-` + 7 base-36 digits of a stable hash of its style (FNV-1a with SplitMix64's finalizer), so one style has one name in every file. Within a file, a style whose name is taken takes more digits. - A function's scoped `@style` is `hl-fn-NAME-HASH`, the hash of the stylesheet, so two functions of one name in two files don't meet. - `htmlang build` reports two files whose different styles got one name (`class-collision`). - The integration tests read the output with its classes numbered in the order they appear (tests/common), so the snapshots are unchanged and change only when styles do. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01Hca8AvqJAmkTymf66PBCGk --- CONTRIBUTING.md | 10 +- DESIGN.md | 18 ++- README.md | 14 +- crates/htmlang-core/src/ast.rs | 2 +- crates/htmlang-core/src/codegen.rs | 183 +++++++++++++++++------ crates/htmlang-core/src/codegen_tests.rs | 38 ++++- crates/htmlang-core/src/diagnostic.rs | 4 + crates/htmlang-core/src/parser.rs | 29 ++-- src/main.rs | 74 ++++++++- tests/common/mod.rs | 140 +++++++++++++++++ tests/regressions.rs | 11 +- tests/snapshots.rs | 107 ++++++------- 12 files changed, 496 insertions(+), 134 deletions(-) create mode 100644 tests/common/mod.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 48d60ff..41879c8 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -40,8 +40,11 @@ Linux, macOS, and Windows. Your change should pass all three. Every CSS property goes through one generic path (its name, its value as written, with `vocab::with_px`'s pixels); only htmlang's layout words and `line-clamp` have code of their own. Generated classes are - `hl-` + a short name; an element's defaults (its layout, - `ElementKind::css`) go with its class, as `:where(.hl-a)`, never a + `hl-` + 7 base-36 digits of `stable_hash` of the style + (`CLASS_DIGITS`), so a style has one name in every file, and + `htmlang build` checks no name stands for two styles; an element's + defaults (its layout, + `ElementKind::css`) go with its class, as `:where(.hl-2k9xq0m)`, never a rule on a global element selector, so `@markdown`/`@raw` HTML and a page embedding a fragment are untouched. The rules are in cascade layers (`LAYERS`): `hl-reset`, `hl-kind` (the defaults), `hl-inside` @@ -100,6 +103,9 @@ Linux, macOS, and Windows. Your change should pass all three. in `docs.rs`. - `editors/vscode/` — VS Code extension. - `tests/snapshots.rs` — integration / snapshot tests for the compiler. + These tests, and `tests/regressions.rs`, read the output with its + hashed classes numbered (`hl-a`, `hl-b`, ..., see `tests/common`), so a + snapshot changes when a style does, not when a hash does. - `tests/regressions.rs` — one test per fixed bug. - `tests/docs.rs` — compiles every example in `DESIGN.md` and `README.md`, and every page in `examples/`: any diagnostic fails it. diff --git a/DESIGN.md b/DESIGN.md index 2799160..1842930 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -734,7 +734,7 @@ own attributes are for it alone. Each style under an element prefix compiles to a rule in CSS's `@scope`, on the element's class, in the layer `hl-inside` (see [CSS](#css)): -`@scope (.hl-a) { :scope td { padding: 8px } }`. `@scope`'s proximity +`@scope (.hl-2k9xq0m) { :scope td { padding: 8px } }`. `@scope`'s proximity rule is what makes the nearest element win. `@scope` is Baseline 2025, supported by Chrome and Edge 118, Safari 17.4 and Firefox 146; an older browser ignores these rules, so the elements inside keep the styles they @@ -1563,8 +1563,18 @@ for a single rule, on its line: @el [animation fade-in 0.3s ease, class=note] Fades in ``` -Each element with styles gets a short generated class, `hl-a`, `hl-b`, ..., -and elements with the same styles share one. The `hl-` prefix is htmlang's: +Each element with styles gets a generated class, and elements with the +same styles share one. A class is named by its style: `hl-` and seven +characters of a hash of what it sets (`hl-2k9xq0m`), so one style has one +name in every file. A fragment put into a page (`--partial` output that a +server returns, or any HTML of one file put into another's page) means +the same there, and its rules restyle nothing of the page's: a class of +both is the same style. Two different styles of one file never share a +name (the later one takes more characters), and `htmlang build` checks +the whole site: two files whose different styles hash to one name are an +error (`class-collision`), which one more attribute on either style +fixes. A function's scoped `@style` is named by the function and a hash +of the stylesheet, `hl-fn-card-1x0a9zq`. The `hl-` prefix is htmlang's: a class of your own that starts with it gets a warning, since the generated rules would apply to it too. Every style goes into the class under its own name, with its value as written (plus pixels, see @@ -1587,7 +1597,7 @@ the specificity: | Layer | Holds | |---|---| | `hl-reset` | The reset | -| `hl-kind` | The defaults of each element's kind, as `:where(.hl-a)` | +| `hl-kind` | The defaults of each element's kind, as `:where(.hl-2k9xq0m)` | | `hl-inside` | The styles an element gets from an element prefix on an element around it (`@td:padding 8`, see [Styling the elements inside](#styling-the-elements-inside)), and functions' scoped `@style` | | `htmlang` | Each element's own styles, and its parent's `children:` styles | diff --git a/README.md b/README.md index 4f7bebb..9ce0a16 100644 --- a/README.md +++ b/README.md @@ -89,11 +89,13 @@ unless you write some, and nothing is added that you didn't ask for. written (`outline 2 solid red` is `outline: 2px solid red`, nothing added). Nothing you write is left out of the page without an error. - **The generated CSS stays on htmlang's elements.** Its classes are - `hl-a`, `hl-b`, ..., in cascade layers (`hl-reset`, `hl-kind`, - `hl-inside`, `htmlang`), so your own CSS wins. An element's defaults (no - heading margins, no list markers, links in the text colour) go with its - own class, so `@markdown` and `@raw` HTML, and a page that embeds - `--partial` output, keep theirs. + named by their styles (`hl-2k9xq0m`), in cascade layers (`hl-reset`, + `hl-kind`, `hl-inside`, `htmlang`), so your own CSS wins. An element's + defaults (no heading margins, no list markers, links in the text colour) + go with its own class, so `@markdown` and `@raw` HTML, and a page that + embeds `--partial` output, keep theirs. One style has one name in every + file, so a fragment a server swaps into a page (with htmx, say) means + the same there and restyles nothing of the page's. - **HTML stays HTML.** An element that isn't about layout or text has its HTML name (`@nav`, `@ul`, `@form`, `@details`, `@strong`, `@em`, `@br`, `@caption`), from a fixed list, so a typo is an error rather than a new @@ -134,7 +136,7 @@ htmlang build src -o dist # compile a whole site | Command | Purpose | |---|---| -| `build [-o out] [--minify] [--strict]` | Compile every `.hl` file under a directory (into `out/` by default), except libraries: files that hold only `@let`s | +| `build [-o out] [--minify] [--strict]` | Compile every `.hl` file under a directory (into `out/` by default), except libraries: files that hold only `@let`s. Two files whose different styles got one class name are an error (`class-collision`) | | `serve [dir\|file] [-p PORT] [--open]` | Dev server with live reload. It serves files, so it answers GET and HEAD (other methods get 405), and reloads pages, not fragments | | `watch [dir\|file] [-o out]` | Recompile on change, without a server | | `check [--format json]` | Report diagnostics without writing output | diff --git a/crates/htmlang-core/src/ast.rs b/crates/htmlang-core/src/ast.rs index 54e103b..e6f245e 100644 --- a/crates/htmlang-core/src/ast.rs +++ b/crates/htmlang-core/src/ast.rs @@ -12,7 +12,7 @@ pub struct Document { pub css_vars: Vec<(String, String)>, pub custom_css: Vec, /// The scoped `@style` of each function called, nested under its - /// `.hl-fn-NAME` class, for `@layer hl-inside`. + /// `.hl-fn-NAME-HASH` class, for `@layer hl-inside`. pub scoped_css: Vec, pub og_tags: Vec<(String, String)>, pub nodes: Vec, diff --git a/crates/htmlang-core/src/codegen.rs b/crates/htmlang-core/src/codegen.rs index c7fdfcf..8ab60b8 100644 --- a/crates/htmlang-core/src/codegen.rs +++ b/crates/htmlang-core/src/codegen.rs @@ -1,5 +1,5 @@ use std::collections::HashMap; -use std::hash::{Hash, Hasher}; +use std::hash::Hash; use crate::ast::*; use crate::vocab::with_px; @@ -17,33 +17,51 @@ const BREAKPOINTS: &[(&str, &str)] = &[ ("2xl", "1536px"), ]; -/// The prefix of every generated class (`hl-a`, `hl-b`, ...), which keeps -/// them out of the author's own class names. +/// The prefix of every generated class (`hl-2k9xq0m`), which keeps them +/// out of the author's own class names. pub const CLASS_PREFIX: &str = "hl-"; -/// Generate short CSS class names: a..z, then aa..a9, ba..b9, ..., z9, then -/// aaa, ... The first character is always a letter; later ones are drawn from -/// [a-z0-9]. The mapping is a bijection, so distinct indices never collide. -pub(crate) fn short_class_name(idx: usize) -> String { - const REST: &[u8] = b"abcdefghijklmnopqrstuvwxyz0123456789"; - // Find the name length: 26 names of length 1, 26*36 of length 2, ... - let mut n = idx; - let mut len = 1; - let mut count = 26usize; - while n >= count { - n -= count; - len += 1; - count = count.saturating_mul(36); - } - let mut tail = Vec::with_capacity(len - 1); - for _ in 1..len { - tail.push(REST[n % 36]); - n /= 36; - } - let mut name = String::with_capacity(len); - name.push((b'a' + n as u8) as char); - name.extend(tail.iter().rev().map(|&b| b as char)); - name +/// How many base-36 digits a class's name has after `hl-`: 7, about 36 +/// bits of its style's hash, so a site with 10,000 different styles has +/// odds of about 6 in 10,000 that two of them share a name, which +/// `htmlang build` reports (`class-collision`). Within one file, a style +/// whose name is taken takes more digits. +pub const CLASS_DIGITS: usize = 7; + +/// A stable hash of `text`: FNV-1a, then SplitMix64's finalizer, so every +/// digit of a name depends on all of the text. It is the same on every +/// platform and in every build, unlike `std`'s hasher. +pub fn stable_hash(text: &str) -> u64 { + let mut hash: u64 = 0xcbf2_9ce4_8422_2325; + for byte in text.bytes() { + hash ^= u64::from(byte); + hash = hash.wrapping_mul(0x0000_0100_0000_01b3); + } + hash = (hash ^ (hash >> 30)).wrapping_mul(0xbf58_476d_1ce4_e5b9); + hash = (hash ^ (hash >> 27)).wrapping_mul(0x94d0_49bb_1331_11eb); + hash ^ (hash >> 31) +} + +/// `digits` base-36 digits of `hash`: `2k9xq0m`. With 13, all of it. +pub fn hash_digits(hash: u64, digits: usize) -> String { + const DIGITS: &[u8] = b"0123456789abcdefghijklmnopqrstuvwxyz"; + let mut n = hash; + (0..digits) + .map(|_| { + let digit = DIGITS[(n % 36) as usize] as char; + n /= 36; + digit + }) + .collect() +} + +/// A class htmlang generated: its name, and the hash of the style it +/// stands for. One style has one name in every file, so a fragment's +/// classes mean the same in any page it goes into. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Class { + pub name: String, + pub style: u64, } /// When a style applies: the at-rule prefixes it is under (their ranks, @@ -254,8 +272,33 @@ fn at_rule(rank: usize, dev: bool) -> String { } } +/// A style as text, for its hash: the defaults of its element's kind, its +/// rules (the at-rules' ranks, the selector prefixes and the declarations +/// of each) and the rules keyed on it, each part ended by a character CSS +/// doesn't have. +fn style_text(defaults: &str, rules: &[(Condition, String)], distinct: &str) -> String { + let mut text = format!("{defaults}\u{1}"); + for (condition, body) in rules { + for rank in &condition.at { + text.push_str(&format!("{rank}\u{2}")); + } + text.push('\u{3}'); + for prefix in &condition.selector { + text.push_str(prefix); + text.push('\u{2}'); + } + text.push('\u{3}'); + text.push_str(body); + text.push('\u{1}'); + } + text.push_str(distinct); + text +} + struct StyleEntry { class_name: String, + /// The hash of the style, which names the class + style: u64, /// The defaults of the element's kind (its layout, a heading's /// `margin:0`, ...), less what its own attributes set. They are written /// as `:where(.hl-a)` in the layer `hl-kind`, so an element prefix @@ -272,10 +315,11 @@ struct StyleEntry { struct StyleCollector { entries: Vec, - /// Maps a pre-hashed style signature to an index into `entries`. - /// Using u64 as the key keeps lookups allocation-free; on the rare case of - /// a hash collision we fall back to a full equality check against the entry. + /// Maps a style's hash to its entries (more than one only when two + /// styles hash alike, which the full comparison tells apart). index: HashMap>, + /// The names given, which a style whose name is taken doesn't get + names: std::collections::HashSet, /// Rules for an element's children, keyed on its class and written in /// the block of an at-rule condition: `(at-rules, selector, body)`, e.g. /// `([md], ":where(.a)>.b", "flex:1;")` (see [`Flow`]). @@ -289,6 +333,7 @@ impl StyleCollector { StyleCollector { entries: Vec::new(), index: HashMap::new(), + names: std::collections::HashSet::new(), keyed: Vec::new(), keyed_seen: std::collections::HashSet::new(), } @@ -314,14 +359,9 @@ impl StyleCollector { return None; } rules.sort_by_cached_key(|(condition, _)| condition.order()); - use std::collections::hash_map::DefaultHasher; - let mut h = DefaultHasher::new(); - defaults.hash(&mut h); - rules.hash(&mut h); - distinct.hash(&mut h); - let sig = h.finish(); - - if let Some(indices) = self.index.get(&sig) { + let style = stable_hash(&style_text(&defaults, &rules, &distinct)); + + if let Some(indices) = self.index.get(&style) { for &idx in indices { let e = &self.entries[idx]; if e.defaults == defaults && e.rules == rules && e.distinct == distinct { @@ -329,18 +369,34 @@ impl StyleCollector { } } } + // The name is the hash's first digits, and more of them when + // another style of this file has those; after all 13, the entry's + // number tells the two apart let idx = self.entries.len(); - let name = format!("{}{}", CLASS_PREFIX, short_class_name(idx)); + let name = (CLASS_DIGITS..=13) + .map(|digits| format!("{}{}", CLASS_PREFIX, hash_digits(style, digits))) + .find(|name| !self.names.contains(name)) + .unwrap_or_else(|| format!("{}{}{}", CLASS_PREFIX, hash_digits(style, 13), idx)); + self.names.insert(name.clone()); self.entries.push(StyleEntry { class_name: name.clone(), + style, defaults, rules, distinct, }); - self.index.entry(sig).or_default().push(idx); + self.index.entry(style).or_default().push(idx); Some(name) } + /// The classes given, in the order they were + fn classes(&self) -> impl Iterator + '_ { + self.entries.iter().map(|e| Class { + name: e.class_name.clone(), + style: e.style, + }) + } + /// All generated rules, in three layers (see [`LAYERS`]): the element /// kinds' defaults in `hl-kind` (`:where(.hl-a)`), the rules for the /// elements an element prefix picks out inside an element (`@td:`), and @@ -628,15 +684,22 @@ pub struct CodegenOptions { /// Generate HTML from a parsed document using the given options. pub fn generate_with(doc: &Document, opts: &CodegenOptions) -> String { + generate_with_classes(doc, opts).0 +} + +/// [`generate_with`], and the classes the document's styles were given, +/// for a check across files that no name stands for two styles. +pub fn generate_with_classes(doc: &Document, opts: &CodegenOptions) -> (String, Vec) { + let mut classes = Vec::new(); let mut html = if opts.partial { - generate_partial_inner(doc, opts.dev) + generate_partial_inner(doc, opts.dev, &mut classes) } else { - generate_full_inner(doc, opts.dev) + generate_full_inner(doc, opts.dev, &mut classes) }; if opts.minify { html = minify_html(&html); } - html + (html, classes) } pub fn generate(doc: &Document) -> String { @@ -762,10 +825,10 @@ fn minify_html(html: &str) -> String { result } -fn generate_full_inner(doc: &Document, dev: bool) -> String { +fn generate_full_inner(doc: &Document, dev: bool, classes: &mut Vec) -> String { // Without `@page` the output is a fragment let Some(page) = &doc.page else { - return generate_partial_inner(doc, dev); + return generate_partial_inner(doc, dev, classes); }; let mut styles = StyleCollector::new(); let mut ctx = GenContext { @@ -880,6 +943,7 @@ fn generate_full_inner(doc: &Document, dev: bool) -> String { String::new() }; let reset_css = reset_css(dev, true, &focus_visible_css); + classes.extend(styles.classes()); format!( "{nl}{nl}{nl}{nl}\ @@ -1022,7 +1086,7 @@ fn reset_css(dev: bool, page: bool, focus_visible_css: &str) -> String { } /// Generate an HTML fragment: body + optional