Skip to content

Diff Engine: Don't emit a changeset for insignificant whitespace changes - #2009

Merged
marcoroth merged 5 commits into
mainfrom
diff-insignificant-whitespace-changes
Aug 5, 2026
Merged

marcoroth merged 5 commits into
mainfrom
diff-insignificant-whitespace-changes

Conversation

@marcoroth

Copy link
Copy Markdown
Owner

This pull request updates the Syntax Tree Diff Engine so that whitespace changes HTML collapses away no longer produce a changeset, and adds an opt-in option for callers that do want to see them.

Outside of whitespace preserving elements, HTML collapses every run of whitespace into a single space. Going from 0 to 1 space or from 1 to 0 changes what gets rendered, but going from 1 to n does not. The diff engine compared HTMLTextNode content byte for byte, so it reported all three the same way:

Herb.diff("<div>Hello World</div>", "<div>Hello     World</div>")
# => #<Herb::DiffResult 1 operation>  (:text_changed)

Reindenting a template therefore produced a changeset with nothing to apply, and Herb::Dev::Runner broadcast a patch for a file that renders identically.

How it works

HTMLTextNode content is now compared with whitespace runs collapsed, so a run that only grows or shrinks is not a difference:

Herb.diff("<div>Hello World</div>", "<div>Hello     World</div>").identical?  # => true
Herb.diff("<div>Hello\n\tWorld</div>", "<div>Hello World</div>").identical?  # => true
Herb.diff("<div>Hello World</div>", "<div>HelloWorld</div>").identical?      # => false
Herb.diff("<div>Hello</div>", "<div>Hello </div>").identical?                # => false

The same applies to the whitespace between elements, which the parser also stores as HTMLTextNodes, so reindenting a nested element is no longer a change:

<div>
  <span>Hello</span>
</div>
<div>
      <span>Hello</span>
</div>

Removing that whitespace entirely still is, and comes through as node_removed.

Inside pre, textarea, script, and style every whitespace change stays significant, so the comparison there remains byte exact. A new is_whitespace_preserving_element() predicate in html_util drives this, and the flag is inherited by everything nested inside such an element, including ERB blocks:

<pre><% if condition %>Hello     World<% end %></pre>

Attribute values and ERB content are untouched, since neither is subject to HTML whitespace collapsing.

One consequence worth calling out: trees_identical is now "no operations were emitted" rather than "the root hashes matched". That keeps the existing identical? == operations.empty? invariant, which would otherwise break the moment an operation is suppressed, and it means Herb::Dev::Runner still returns early on formatting only saves instead of broadcasting an empty patch.

Ruby API

Herb.diff(old_source, new_source, detect_whitespace_changes: true)
# => #<Herb::DiffResult 1 operation>  (:whitespace_changed)

JavaScript API

Herb.diff(oldSource, newSource, { detect_whitespace_changes: true })

Rust API

herb::diff_with_options(old_source, new_source, &DiffOptions { detect_whitespace_changes: true })

Java API

Herb.diff(oldSource, newSource, new DiffOptions().detectWhitespaceChanges(true));

Resolves #1983

@marcoroth marcoroth changed the title Herb: Don't emit a changeset for insignificant whitespace changes Diff Engine: Don't emit a changeset for insignificant whitespace changes Aug 5, 2026
@github-actions github-actions Bot added wasm WebAssembly build and bindings typescript TypeScript source across the javascript/ packages c C source for the core parser, lexer, and AST node @herb-tools/node native Node.js addon c-extension Ruby C extension in ext/ rbs RBS type signatures in sig/ rust Rust bindings and the Herb Rust crate java Java bindings and the org.herb package core @herb-tools/core shared AST nodes, interfaces, and utilities cpp C++ source, primarily the WebAssembly bindings dev-server-client Browser Client for the Herb Dev Server diff Herb Syntax Tree Diff Engine labels Aug 5, 2026
@github-actions

github-actions Bot commented Aug 5, 2026 •

Copy link
Copy Markdown

🌿 Interactive Playground and Documentation Preview

A preview deployment has been built for this pull request. Try out the changes live in the interactive playground:


🌱 Grown from commit aac4e6b


✅ Preview deployment has been cleaned up.

@github-actions github-actions Bot added the playground The Herb playground web app label Aug 5, 2026
@pkg-pr-new

pkg-pr-new Bot commented Aug 5, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/@herb-tools/formatter@2009
npx https://pkg.pr.new/@herb-tools/language-server@2009
npx https://pkg.pr.new/@herb-tools/linter@2009

commit: aac4e6b

@marcoroth
marcoroth marked this pull request as ready for review August 5, 2026 11:05
@marcoroth
marcoroth merged commit 2c03114 into main Aug 5, 2026
34 checks passed
@marcoroth
marcoroth deleted the diff-insignificant-whitespace-changes branch August 5, 2026 11:15
@marcoroth marcoroth added this to the v1.0.0 milestone Aug 8, 2026

This branch was successfully deployed

1 active deployment
herb-tools (Preview) — aac4e6b2 Deployed Aug 5, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c C source for the core parser, lexer, and AST c-extension Ruby C extension in ext/ core @herb-tools/core shared AST nodes, interfaces, and utilities cpp C++ source, primarily the WebAssembly bindings dev-server-client Browser Client for the Herb Dev Server diff Herb Syntax Tree Diff Engine java Java bindings and the org.herb package node @herb-tools/node native Node.js addon playground The Herb playground web app rbs RBS type signatures in sig/ rust Rust bindings and the Herb Rust crate typescript TypeScript source across the javascript/ packages wasm WebAssembly build and bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Diff: Should not emit a changeset if its a non-significant whitespace change

1 participant