Refactor to LET — PR 4: promotable literals + drop-input UX polish (spec 0008)#214
Merged
Merged
Conversation
Final slice of the /Refactor feature. Adds literal promotion and
refines the dialog status bar.
- Literal tokenizer (RefactorEngine.WalkLiterals): walks formula text,
masking cell-ref token spans via CellRefExtractor.MatchSpans and
skipping strings / sheet qualifiers, and yields numeric, string, and
boolean literals in source order. Numerics dedupe by parsed value
(0.20 and 0.2 collapse, first spelling wins); strings by un-escaped
value; booleans case-insensitively. Each unique literal becomes a
default-off RefactorPromotableRow(kind: Literal).
- Promoting a literal materialises an input binding whose RHS is the
original token text; the new literal rewrite pass (RewriteLiterals,
run after the cell-ref and identifier passes) swaps every occurrence
for the binding name.
- Drop-input UX: dropped promoted literals / named ranges fall out of
the rewrite map, restoring the original token inline (covered by
tests).
- Status bar shows a summary count ("N inputs, M promotable, K
calculation binding(s)") plus a merge note, with validation errors
taking priority.
Literals surface in source order (the issue's example listing groups
them differently), consistent with the first-occurrence convention used
elsewhere in the engine.
Closes #205
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Final slice of the Refactor to LET feature (spec 0008). Adds literal promotion to the promotable section and polishes the drop-input UX + dialog status bar.
Closes #205 · Part of #201
What changed
Literal tokenizer (
RefactorEngine.WalkLiterals)Walks the formula text in source order, masking cell-ref token spans (new
CellRefExtractor.MatchSpans) so the row digits ofA1aren't mistaken for a literal, and skipping string contents / sheet qualifiers. Yields:100,0.20,1.5e-10) — dedupe by parsed value, so0.20and0.2collapse to one row (first spelling preserved). No leading sign is captured, soA1*-5correctly promotes5and keeps-inline.""handled); token keeps the original escaped spelling.TRUE/FALSEcase-insensitive, excluding function calls (TRUE() and sheet qualifiers.Each unique value becomes a default-off
RefactorPromotableRow(kind: Literal)with an occurrence count.Promotion + rewrite
Promoting a literal materialises an input binding whose RHS is the original token text (preserves
0.20vs0.2). A newRewriteLiteralspass — run after the cell-ref and identifier passes so a promoted-string binding name is never re-scanned as an identifier — swaps every occurrence for the binding name.Drop-input UX
Unticking a promoted literal or named range drops it from the rewrite map, restoring the original token inline everywhere (no warning even if it reintroduces duplication). Covered by new tests.
Status bar
Now shows a summary count —
3 inputs, 2 promotable, 1 calculation binding— plus a· N mergednote, with validation errors taking priority.Note on ordering
Literals are surfaced in source order, matching the first-occurrence convention used everywhere else in the engine. For the manual-test formula this means
50appears between"high"and"medium"rather than after"low"as the issue's example listing sketched — the source-order placement is the principled one.Tests
12 new
RefactorEngineTestscover: numeric/string(with"")/boolean promotion, two distinct numerics staying distinct, value-dedupe (0.20/0.2), digits-inside-identifier safety, the issue's manual-example token set, drop-input restoring literals + named ranges, existing-LET literal scoping (value-binding RHS excluded), and round-trip safety viaLetParser+LetToLambdaBuilder. Two pre-existing promotable tests were narrowed to their kind (their formulas now also surface a literal). Full suite: 980 passing.Manual Excel test (for Tim)
Type
=IF(A1 > 100, "high", IF(A1 > 50, "medium", "low")) + IF(B1 > 100, 1, 0), run/Refactor. The promotable section should list100(2 uses),"high",50,"medium","low",1,0. Promote100only, Save, and confirm both100s were replaced and the formula evaluates identically.🤖 Generated with Claude Code