[cdytotoro-ctrl]

Terse mode was only enabled when the Doctype node was reached during the AST walk, so tags compiled before the doctype in document order (mixins declared above 'doctype html', content above the doctype, or mixins in a child template rendered through an extends layout) compiled boolean attributes in non-terse mode (Fixes #1918).

The compiler now pre-applies the first doctype found in the AST before walking it, using the same logic as visitDoctype, which re-applies the same settings later without any effect. Doctypes other than html5 and templates without a doctype keep the previous behavior.

Tests: new test case mixin-above-doctype reproduces the mixin-after-extends scenario from the issue and asserts terse boolean attributes.

[rollingversions]

There is no change log for this pull request yet.

Create a changelog

[seebaermichi]

Thanks for picking up this old one! I tested it against master (c323ed3e), npm pug@3.0.4 and this branch (23b49fb3), with each template rendered on all three.

#1918 is fixed in every form I tried. On master and 3.0.4 these all render selected="selected" / checked="checked" under doctype html; on this branch they render terse:

• a mixin defined and called above doctype html
• a mixin from an include above doctype html, called below it (the report)
• a mixin in a child template that extends a layout with doctype html (the 2020 comment), rendered with renderFile
• input(checked) above doctype html

Changes I'd ask for:

1. Which doctypes the pre-pass finds doesn't follow a clear rule. findFirstDoctype walks nodes, block and elseBlock. No package on master uses elseBlock. Conditional keeps its branches in consequent/alternate and Each its else in alternate, so neither is walked, although the comment says "conditionals". A doctype in a when branch, an included file or a mixin body is found. In each case the template starts with input(checked), then the doctype:
   • doctype html only in an each … else branch: leading tag unchanged (<input checked="checked"/>)
   • doctype xml under if false, then doctype html at the top level: the leading tag becomes terse html, so the if branch was skipped
   • doctype html only in a case/when branch, or only in an included file: the leading tag becomes <input checked>
2. A doctype in a mixin that is never called now changes the tags above it. The template input(checked) / mixin unused / doctype xml (indented, the mixin's body) / input(checked) / doctype html / input(checked), one line each, renders <input checked="checked"/><input checked="checked"></input><!DOCTYPE html><input checked> on master and <input checked="checked"></input><input checked="checked"></input><!DOCTYPE html><input checked> here. The first input now gets xml form from a doctype that is never output. I'd suggest deciding which doctypes count (for example, none inside a mixin definition or an untaken branch) and pinning that rule with a test case for each kind of node.
3. The description says "doctypes other than html5 … keep the previous behavior". They don't: input(checked) above doctype xml (with a doctype html further down) rendered <input checked="checked"/> on master and renders <input checked="checked"></input> here. That is consistent with the fix, but the description and the changelog should say so.
4. The new test case doesn't cover extends. The description says mixin-above-doctype reproduces the mixin-after-extends scenario, but the case is a single file with the mixin above doctype html. A case with a layout and a child template would pin the form most users hit. (The branch already handles it, see above.)

Output changes worth naming in the changelog, all because the first doctype now applies from the first tag on:

• Content above the doctype is compiled with that doctype's mode (input(checked) above doctype html renders <input checked>).
• With several doctypes, the first one now also governs the tags above it; each later doctype still governs the tags after it, as before.
• A template doctype now overrides the doctype option for the tags above it. With {doctype: 'xml'} and doctype html in the template, the leading input(checked) rendered <input checked="checked"></input> on master and renders <input checked> here; with {doctype: 'html'} and doctype xml it is the other way round. Previously the option governed until the template's doctype. I think the new behaviour is the more sensible one, but it is a visible change for anyone who passes the option.

Tests:

• Full suite: 649/651. The 2 failures are the same as on master (V8 error-message wording in error.reporting.test.js).
• run.test.js and run-es2015.test.js: 117/117.
• The new mixin-above-doctype case fails without the change in pug-code-gen (selected="selected") and passes with it.
• Nera's test suites and site builds against this branch are green, with byte-identical output.

Rolling Versions still says there is no changelog entry. Something like this for pug-code-gen would do, plus the output changes above: "Apply the template's first doctype from the start of compilation, so tags and mixins compiled before it use its terse/xml mode (#1918)".

With 1, 2 and the changelog sorted out, this looks good to merge to me.