Skip to content

Use one-based positions in theme check JSON output - #8972

Merged
gonzaloriestra merged 2 commits into
mainfrom
gonzalo/theme-check-one-based-json
Oct 23, 2026
Merged

gonzaloriestra merged 2 commits into
mainfrom
gonzalo/theme-check-one-based-json

Conversation

@gonzaloriestra

@gonzaloriestra gonzaloriestra commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Theme Check JSON uses zero-based positions while terminal output and common source-reading tools use one-based line numbers. Matching those conventions avoids conversions for agents and other consumers. Slack discussion.

WHAT is this pull request doing?

Report one-based startRow, startColumn, endRow, and endColumn through both --json and --output=json, and require positive integers in the result schema. Update the shared JSON convention and generated help.

This is a breaking change: consumers must stop adding 1 to these positions.

How to manually test your changes?

Use a theme with a known Theme Check offense and run:

pnpm shopify theme check --path /path/to/theme --output=text
pnpm shopify theme check --path /path/to/theme --json
pnpm shopify theme check --path /path/to/theme --output=json

Compare the JSON positions with the source in your editor and the line displayed in the text snippet. The first line and column are 1 in JSON; both JSON flags should report the same positions.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact

@github-actions github-actions Bot added the Area: @shopify/cli @shopify/cli package issues label Oct 21, 2026
@gonzaloriestra

Copy link
Copy Markdown
Contributor Author

/snapit

@github-actions github-actions Bot added no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users. and removed Area: @shopify/cli @shopify/cli package issues labels Oct 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🫰✨ Thanks @gonzaloriestra! Your snapshot has been published to npm.

Built from 05aabdab8c5e3c742391618db260fd5dcf0fe52a. Workflow run.

Test the snapshot by installing your package globally:

pnpm i -g --@shopify:registry=https://registry.npmjs.org @shopify/cli@0.0.0-snapshot-20261021083201

Caution

After installing, validate the version by running shopify version in your terminal.
If the versions don't match, you might have multiple global instances installed.
Use which shopify to find out which one you are running and uninstall it.

@gonzaloriestra
gonzaloriestra marked this pull request as ready for review October 21, 2026 08:58
@gonzaloriestra
gonzaloriestra requested review from a team as code owners October 21, 2026 08:58
@gonzaloriestra
gonzaloriestra force-pushed the gonzalo/theme-check-one-based-json branch from 08f9916 to c8dafc4 Compare October 23, 2026 08:35

@karreiro karreiro left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for this PR, @gonzaloriestra!

🎩 LGMT!

Before:
Image

After:
Image

@gonzaloriestra
gonzaloriestra added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit c35b60b Oct 23, 2026
28 of 29 checks passed
@gonzaloriestra
gonzaloriestra deleted the gonzalo/theme-check-one-based-json branch October 23, 2026 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog This PR doesn't include a changeset entry. Is an internal only change not relevant to end users.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants