Skip to content

fix(changelog): fall through to the next parser when a commit field is missing - #1600

Open
YuriNachos wants to merge 1 commit into
orhun:mainfrom
YuriNachos:fix/parser-fallthrough-missing-field
Open

fix(changelog): fall through to the next parser when a commit field is missing#1600
YuriNachos wants to merge 1 commit into
orhun:mainfrom
YuriNachos:fix/parser-fallthrough-missing-field

Conversation

@YuriNachos

Copy link
Copy Markdown

Closes #1598.

Problem

When a commit parser references a field that is genuinely absent on a commit (e.g. remote.pr_labels on a local / non-PR commit where remote is None), the parser returned FieldError and aborted the whole parse — so a single field-based parser could prevent any later (catch-all) parser from grouping the commit. Reported in #1598: a local commit gets skipped by the commit parsers if one of them looks for a field that does not exist on it.

Fix

Distinguish a missing field from a present-but-unsupported-type field:

  • the field resolves to None because the path is absent → skip only this field criterion and let the rest of the parser (and the remaining parsers) decide;
  • the field is present but has an unsupported type (e.g. a bare Value::Object) → still error, exactly as before.

So a local commit no longer aborts the chain when a parser looks for remote.pr_labels; the catch-all parser can still group it.

Test

parse_commit_missing_field_falls_through_to_next_parser — a commit with remote = None and two parsers (field remote.pr_labels, then a catch-all) now groups via the catch-all instead of erroring. The existing parse_commit_field (present-but-Object → error) still passes, preserving that contract.

cargo test -p git-cliff-core commit::test → 9 passed; cargo clippy --tests -D warnings clean.

@YuriNachos
YuriNachos requested a review from orhun as a code owner August 7, 2026 12:25
…s missing

When a commit parser specified both `field` and `pattern` and the named
field could not be resolved on a commit (e.g. `remote.pr_labels` on a
local / non-PR commit where `remote` is None), `Commit::parse` aborted
the whole parser chain with `AppError::FieldError`. Later catch-all
parsers were never tried, so the commit was silently dropped from the
changelog ("N commit(s) were skipped due to field error(s)").

The field resolution collapsed two different situations into the same
`None`: a genuinely missing field (`tera::dotted_pointer` returned
`None`) and a present-but-`Value::Object` field (mapped to `None` by the
`and_then`). Both hit the `None =>` arm, which returned from the entire
`parse()`.

Distinguish the two with a single `dotted_pointer` lookup
(`field_present = field_value.is_some()`). In the `None =>` arm, a
present `Value::Object` field still returns the identical `FieldError`
(message text unchanged), while a genuinely missing field skips only
that parser so the rest of the chain can still match the commit.

Fixes orhun#1598.

Signed-off-by: Yurii Chukhlib <yurii.chukhlib@viber.com>
@YuriNachos
YuriNachos force-pushed the fix/parser-fallthrough-missing-field branch from 5b3ca31 to 3a63572 Compare August 8, 2026 11:20
@YuriNachos

Copy link
Copy Markdown
Author

Re-triggered CI. For the record, the red Test suite job is not a test failure:

==> Verifying GPG signature integrity
gpg: Signature made Thu Jul  9 01:37:57 2026 UTC
gpg:                using RSA key 27034E7FDB850E0BBC2C62FF806BB28AED779869
gpg: Can't check signature: No public key
==> Could not verify signature. Please contact Codecov if problem continues

Every cargo test step in that job reported test result: ok; the job only fails at the codecov/codecov-action upload step, which cannot fetch the Codecov public key. The same job is red on main for the same reason, so it is not specific to this branch. All the fixture jobs, Check, Lints, Formatting, Doctests and CodeQL are green.

@YuriNachos

Copy link
Copy Markdown
Author

The "Test suite" check is red only on the "Upload reports to codecov" step (codecov-action in TOKENLESS fork-upload mode, exited 1 with CC_FAIL_ON_ERROR). The actual Rust test run is green — 0 failures across the suite; Check / Lints / Doctests / Test fixtures all pass. This is an intermittent codecov tokenless-upload infra flake unrelated to the parser-fallthrough change. Rerunning the job (maintainer) or a push re-trigger should clear it.

@YuriNachos

Copy link
Copy Markdown
Author

A quick check-in on this one: the fall-through behaviour matches what #1598 described — a local commit with remote = None now skips just the remote.pr_labels criterion instead of aborting the parser chain, and the catch-all grouping test covers it. Is anything else needed from my side — extra fixture coverage, a rebase onto main, or changes to how the missing-field case is documented? For clarity, the red Test suite job is the codecov tokenless-upload step I detailed in my earlier comment; the Rust tests themselves pass. Happy to adjust whatever makes this easiest to land.

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.

local commit gets skipped by commit parsers if looking for label field

1 participant