feat(cli)!: show position of error in commit input #633#4629
feat(cli)!: show position of error in commit input #633#4629escapedcat wants to merge 35 commits into
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
|||||||||||||||||||||||||||||
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
PR Code Suggestions ✨Explore these optional code suggestions:
|
|||||||||||||||||
226cf37 to
88d04d7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@knocte wdyt? |
I love this! But IMO:
|
I'm worried this would be a breaking change and mess up whatever people do with current output format
As it was described in the issue and also as I mostly know it from other linters I guess |
I figured, but think about it: by being a flag this feature would be not discoverable at all, most people will not use it because they don't know about it. As a consequence of this, at some point you will think of making it the default, and then the breaking change will happen. Why not make the breaking change already? Just do a higher version in the bump. And let people file bugs, we'll fix them?
In the issue they use an example of something that has known length (the string "thisDoesNotExist") and so the compiler prints as many
Funny you say that because I spotted some compiler errors from typescript, pasted by people into issues, and all I saw is just a single |
7729dae to
1b2ab42
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Wow, copilot feedback is really good |
13def8c to
720e751
Compare
Adds a new --show-position CLI option that displays a position indicator (~~~) under the commit input to show exactly where the error occurs, similar to TypeScript's red squiggly lines. This helps users quickly identify the problematic part of their commit message. Features: - Add optional start/end position fields to LintRuleOutcome and FormattableProblem - Add getRulePosition() helper to calculate error positions for various rules - Add showPosition option to FormatOptions - Add --show-position CLI flag (opt-in, default false) - Add tests for position indicator in format package - Update config-conventional tests to use toMatchObject for backward compatibility
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
header.indexOf(parsed.subject) returns the wrong occurrence when type and subject share text. For headers like "foo: foo" the caret landed on the type rather than the subject for every subject-* rule. Subject sits at the end of the header in every conventional-commits grammar, so lastIndexOf is robust against type/scope text appearing inside the subject string and still works for custom parser presets. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous logic short-circuited via raw.startsWith(parsed.type), so any custom parser preset whose header pattern doesn't begin with the type token (e.g. ----feat: subject) caused type-* rules to return no position. Search for the type within the header so the position is computed correctly regardless of how the headerPattern places the type token. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
raw.indexOf("\n\n") and raw.lastIndexOf("\n\n") never match
"\r\n\r\n", so body-* and footer-* rules returned no position for
Windows-style commit messages.
Normalize raw line endings to \n once at the top of getRulePosition.
The formatter already normalizes the same way before splitting into
lines, so positions still align with the rendered input.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
subject-exclamation-mark validates the breaking-change "!" before the colon, not the subject text itself. Grouping it with the subject-* rules made the caret land on the subject string, which is misleading output for the rule that fires when "!" is present or missing. Split it into its own case: when "!" is present in the header point at it; otherwise point at the colon (where "!" would belong). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The format API docs added showPosition but didn't mention that callers must populate start/end on each problem for the caret to render. Programmatic users of @commitlint/format had no way to know which fields wire up the new behavior. Document both fields on the Problem type, including a note that they are required for the indicator to render. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…pt-out The new --show-position flag was only covered by the help-snapshot assertion, which catches changes to the help text but not regressions in the yargs default wiring or formatter integration. Add two CLI-level tests: - default invocation prints the caret on failure - --no-show-position suppresses the caret Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… colon
header.indexOf("!") matched any "!" in the header, including ones
inside the subject text. For "feat: hello! world" the caret pointed
at the bang inside the subject rather than at the colon position
where the breaking-change marker is missing.
Find the colon first, then check whether "!" sits immediately
before it — that's the only position where the conventional-commits
breaking-change marker is valid.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Both body-leading-blank and footer-leading-blank had a fallback that searched raw for "\n\n", which can match a paragraph break *inside* the body and put the caret on the wrong line. The rule fires for the leading blank specifically, so the caret should always point at the section boundary. - body-leading-blank: point at header.length (header/body boundary). - footer-leading-blank: locate the footer in raw and point at the character immediately before it (body/footer boundary). Tighten the existing tests to assert the expected offset rather than just toBeDefined, plus add a regression test for a body that contains its own "\n\n" paragraph break. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When a commit has no body section at all, raw.indexOf("\n\n")
returned -1 and the rule reported no position even though it had
fired. Fall back to the end of the header so the caret lands where
the missing body would belong.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
parsed.body / parsed.footer may have been trimmed or normalized by the parser, so bodyStart + parsed.body.length can exceed the actual raw range. Clamp to raw.length so the end Position never points past the actual input. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The earlier "export shared Position type" refactor missed the
inline { line, column, offset } shapes in LintRuleOutcome. Replace
them with the exported Position type so the public surface uses a
single named shape.
Also document the units on Position itself: line/column are
1-indexed, offset is 0-indexed character (not display-width)
positions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The new --show-position flag was wired up in code and the help snapshot but missed in docs/reference/cli.md. Add it to the options listing so the rendered docs match the live --help output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…tern The fix that switched type/subject lookup from hard-coded offsets to header-aware searching wasn't covered by a positive position assertion against a non-default parserOpts.headerPattern. Add a test using the type-scope-subject grammar (the same one already used elsewhere in the suite) to lock in the parser-aware behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous loop did independent toContain checks, which would incorrectly pass for scrambled output. Walk the lines forward through stdout and require each next line to appear at or after the previous one's end, so the test fails if the formatter ever reorders the input lines. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the per-rule switch with a small set of field span helpers plus three exact-character special cases (subject-exclamation-mark, body-leading-blank, footer-leading-blank). Field is inferred from the rule-name prefix, which removes the per-new-rule maintenance tax and keeps behavior consistent across the type/scope/subject/header/body/ footer rule families. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the default-on rollout from "feat!: enable position indicator by default". The output-format change now ships behind --show-position so downstream consumers that parse commitlint output are not disrupted. The flag and its --no-show-position counterpart remain available; the caret renders identically when enabled. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace toMatchObject with toEqual and pin the start/end values for each conventional-config error fixture. toMatchObject silently ignored the new position fields, leaving them unverified end-to-end; the explicit expectations lock in field-level position behavior across type/subject/header/body/footer rules. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reverts the opt-in rollout. The caret renders by default; the field- level rewrite has narrowed the bug surface enough that the interactive-UX win outweighs the small risk of disrupting downstream output parsers, and shipping it on in this release avoids a separate default-flip in a future major. Disable with --no-show-position on the CLI or showPosition: false in formatOptions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (blank === -1) return undefined; | ||
| const start = blank + 2; |
| if (blank === -1) return undefined; | ||
| const start = blank + 2; |
| } | ||
| if (ruleName === "footer-leading-blank") { | ||
| if (!parsed.footer) return undefined; | ||
| const footerStart = raw.indexOf(parsed.footer); |
| ruleName: string, | ||
| parsed: ParsedCommit, | ||
| ): { start: Position; end: Position } | undefined { | ||
| const raw = (parsed.raw || "").replace(/\r\n/g, "\n").replace(/\r/g, "\n"); |
| const parenStart = header.indexOf(`(${parsed.scope})`); | ||
| const offset = | ||
| parenStart >= 0 ? parenStart + 1 : header.lastIndexOf(parsed.scope); | ||
| if (offset === -1) return undefined; | ||
| return span(raw, offset, offset + parsed.scope.length); |
| const padding = " ".repeat(prefixLength); | ||
| const caret = "^"; | ||
| const spacesBefore = Math.max(0, problemWithPosition.start.column - 1); | ||
| const text = padding + " ".repeat(spacesBefore) + caret; |
|
I'm sad this didn't make it to v21.x. For 22.x I guess? |
|
Yeah, will pick it up again eventually |
Closes #633.
Adds a position indicator (
^) under the commit input pointing atwhere each linting error occurs, similar to how TypeScript / Rust
point at the offending span. For multi-line commits the caret renders
under the failing row (body / footer rules included), not at the end
of input.
$ echo 'foo: not good' | commitlint
⧗ input: foo: not good
^
✖ type must be one of [...] [type-enum]
Changes
--show-positionCLI flag (defaulttrue); equivalentshowPositionoption in@commitlint/format. Disable with--no-show-positionorshowPosition: false.LintRuleOutcomeandFormattableProblemgain optionalstart/endPositionfields, exported from@commitlint/types.@commitlint/lintcomputes positions field-by-field (type / scope /subject / header / body / footer span based on the rule prefix),
with exact-character placement for
subject-exclamation-mark,body-leading-blank, andfooter-leading-blank.@commitlint/formatrenders the caret under the failing line andindents multi-line input continuation under the input column.
BREAKING CHANGE
The position indicator is shown by default and an extra line is
inserted between the input and the problem list. Tools that scrape
commitlint stdout by line number need updating, or can disable the
indicator with
--no-show-position/showPosition: false.Notes
body / footer rather than computed from the default
type(scope): subjectlayout, so customparserOpts.headerPatternvalues still get a caret. CRLF input is normalized.
parserOpts.commentCharfilters comment lines, offsets computedagainst
parsed.rawcan drift relative to the rendered input.