Skip to content

Liquid assignment slices#44

Open
sanket0896 wants to merge 1 commit intomasterfrom
cursor/liquid-assignment-slices-9938
Open

Liquid assignment slices#44
sanket0896 wants to merge 1 commit intomasterfrom
cursor/liquid-assignment-slices-9938

Conversation

@sanket0896
Copy link
Copy Markdown

@sanket0896 sanket0896 commented Mar 11, 2026

User description

Add splitAssignmentsByVariable function to split Liquid templates by variable assignments, enabling granular processing of templates.

Slack Thread

Open in Web Open in Cursor 

Generated description

Below is a concise technical summary of the changes proposed in this PR:
Enable splitting Liquid templates by assigned variable through the new splitAssignmentsByVariable helper, which parses via the factory’s Liquid engine and prunes AST branches so downstream processors can target each variable slice. Add regression coverage that exercises nested control flow, raw blocks, and ordering expectations to keep the per-variable slicing behavior stable.

TopicDetails
Assignment Tests Add fixture-driven expectations plus targeted unit cases that cover branch pruning, raw blocks, alternative assign tags, ordering, and case/when structures for the new slicing helper.
Modified files (3)
  • test/fixtures/sample-liquid-input.liquid
  • test/fixtures/sample-liquid-output.liquid
  • test/split-assignments.js
Latest Contributors(0)
UserCommitDate
Assignment Slicing Provide per-variable slicing via splitAssignmentsByVariable, which walks the parsed Liquid AST, keeps assign tags/control flow that touch a target, and renders each filtered branch back to Liquid text so templates can be processed one variable at a time.
Modified files (2)
  • index.js
  • src/split-assignments.js
Latest Contributors(2)
UserCommitDate
deev1207feat-dynamic-table-val...March 20, 2025
champdeev@gmail.comModified-validations.j...March 10, 2025
This pull request is reviewed by Baz. Review like a pro on (Baz).

- Implement splitAssignmentsByVariable() using existing parser AST
- Support assign, assignVar, parseAssign tags
- Handle if/elsif/else, unless, for, case, capture, raw blocks
- Prune empty branches when pruneEmptyBlocks is true
- Preserve control-flow structure per variable
- Add unit tests and sample fixtures
- Export from index.js

Co-authored-by: Sanket Mishra <sanket0896@users.noreply.github.com>
@cursor
Copy link
Copy Markdown

cursor bot commented Mar 11, 2026

Cursor Agent can help with this pull request. Just @cursor in comments and I'll start working on changes in this branch.
Learn more about Cursor Agents

@sanket0896 sanket0896 marked this pull request as ready for review March 11, 2026 15:30
Comment on lines +122 to +133
if (t.name === 'comment' && !options.includeComments) continue
if (t.name === 'raw') {
if (keepNonAssign) result.push(t)
continue
}

const filtered = filterControlFlow(t, targetVar, options)
if (filtered) {
if (pruneEmptyBlocks && isEmptyBlock(filtered)) continue
result.push(filtered)
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderTemplateToLiquid doesn't handle the comment tag, so comments preserved by filterTemplates when includeComments: true (line 122) fall through to the final return ''. As a result includeComments never renders comment tags and the output Liquid drops them. Can we add an if (name === 'comment') return (token && token.raw) || '' branch (similar to assigns)?

Finding type: Logical Bugs | Severity: 🟠 Medium


Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents:

In src/split-assignments.js around lines 122 to 344, the renderTemplateToLiquid function
fails to handle the 'comment' tag even though filterTemplates can keep comments when
includeComments is true; as a result comments are dropped from the output. Modify
renderTemplateToLiquid to add a branch for when name === 'comment' that returns (token
&& token.raw) || '' (similar to the existing assign-handling branch) so comment tags are
preserved in the rendered Liquid.

Comment on lines +253 to +264
function renderTemplateToLiquid (template) {
if (!template) return ''
if (template.type === 'html') return template.raw || template.value || ''
if (template.type === 'value') return (template.token && template.token.raw) || ''
if (template.type !== 'tag') return ''

const name = template.name
const token = template.token

if (['assign', 'assignVar', 'parseAssign'].indexOf(name) >= 0) {
return (token && token.raw) || ''
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderTemplateToLiquid only emits raw output for tag names ['assign','assignVar','parseAssign'], but filterTemplates can retain any tag listed in options.assignTags. A custom assignment tag (e.g. set) survives filtering but hits renderTemplateToLiquid's default case and becomes an empty string, so the per-variable slice silently loses the assignment; can we make rendering use options.assignTags (or a shared list) so filtered custom assignment tags are re-serialized instead of dropped?

Finding type: Logical Bugs | Severity: 🔴 High


Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents:

In src/split-assignments.js around lines 253 to 264, the renderTemplateToLiquid function
hardcodes ['assign','assignVar','parseAssign'] and so drops custom assignment tags that
passed filterTemplates. Make rendering aware of the configured assignTags by refactoring
renderTemplatesToLiquid and renderTemplateToLiquid to accept an options (or assignTags)
parameter and use options.assignTags instead of the hardcoded array. Propagate this new
parameter from the top-level splitAssignmentsByVariable where
renderTemplatesToLiquid(filtered) is called (change to renderTemplatesToLiquid(filtered,
opts)), and update all internal calls to pass the options down so custom assignment tags
are serialized rather than omitted.

Comment on lines +266 to +276
const impl = template.tagImpl
if (name === 'if' && impl && impl.branches && impl.branches.length > 0) {
let out = (token && token.raw) || ''
out += renderTemplatesToLiquid(impl.branches[0].templates || [])
for (let i = 1; i < impl.branches.length; i++) {
const b = impl.branches[i]
out += `{% elsif ${b.cond} %}`
out += renderTemplatesToLiquid(b.templates || [])
}
if ((impl.elseTemplates || []).length > 0) {
out += '{% else %}'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renderTemplateToLiquid hardcodes out += {% elsif ${b.cond} %}, so original {%- elsif … -%} or {% elsif … -%} whitespace-control modifiers are lost. This drops the trim directive and can change rendered HTML/newlines, violating keepNonAssign's intent to preserve original control-flow syntax. Can we store each branch's raw token when parsing and reuse it here instead of emitting a generic {% elsif … %}?

Finding type: Logical Bugs | Severity: 🔴 High


Want Baz to fix this for you? Activate Fixer

Other fix methods

Fix in Cursor

Prompt for AI Agents:

In src/split-assignments.js around lines 266 to 276, the renderTemplateToLiquid function
emits a hardcoded `{% elsif ${b.cond} %}`, which loses any original whitespace-control
(e.g. {%- elsif … -%}) and breaks preservation of original control-flow formatting.
Modify the code so it uses the branch's original raw token when available: (a) in
filterControlFlow where newBranches are created (around lines ~147-166), copy the
original branch token (e.g. b.token or b.token.raw) onto each new branch object you
push; (b) in renderTemplateToLiquid (lines 266-276), replace the hardcoded `{% elsif ...
%}` emission with using b.token.raw (falling back to the current synthetic `{% elsif
${b.cond} %}` if b.token or b.token.raw is missing). This preserves any trim/whitespace
modifiers from the source.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants