Commitlint: Slash in subject results in undesired results

Created on 6 Feb 2019  Â·  7Comments  Â·  Source: conventional-changelog/commitlint

I use this config:

{
  "extends": ["@commitlint/config-conventional"],
  "rules": {
    "scope-case": [0],
    "subject-case": [2, "always", "sentence-case"]
  }
}

If I write something like this, then the cli throws an error: chore: Update @angular/core.

â§—   input: chore: Update @angular/core
✖   subject must be sentence-case [subject-case]
✖   found 1 problems, 0 warnings

Expected Behavior

The subject should be valid.

Current Behavior

The toCase function in case.js will be called two times:

  • Update @angular
  • core

So there is somewhere a split by the slash, which results in a validation error because core starts with an lower case.

Affected packages

  • [x] cli
  • [ ] core
  • [ ] prompt
  • [ ] config-angular

Possible Solution

I think there should be no split in the subject. Maybe there is a connection to https://github.com/conventional-changelog/commitlint/issues/291

bug

All 7 comments

Ah I see, thanks for the report! I think this might be related to a fix we implemented to allow slashes in the scope part (#341). We could change it a bit to allow slashes like that _before_ the type-subject delimiter. Or we might be able to solve this using options provided to ensure/toCase method, like { allowSlashes: true|false }. What do you think about this @marionebl @escapedcat?

Let me create a branch/pr with the scenario described here, after that we can find a way to fix this.

[...] allow slashes like that _before_ the type-subject delimiter.

I feel like this might be a way to go.
Adding just another option to take care of to support another edge-case seems cumbersome to me.

So this would still be checked in the future:

chore(your/component): things and @angular/core

If sentence-case is chosen it should be corrected to:

chore(Your/Component): Things and @angular/core

Correct?

Adding just another option to take care of to support another edge-case seems cumbersome to me.

Yeah, I feel the same. But it's one of the options available 😄

Correct?

Yeah, exactly! I should definitely add some examples next time too 😄

Hmm, wait, I don't think our solution would work @escapedcat. The idea works fine for ensure with the full commit. But I double checked the lint package, how it executes the rules and what data they are passed. Turns out, the scope only gets the scope and subject gets the subject only.

One thing I'll try out now is to move the delimiter-split-logic to the scope-case rule only. With that, we remove the misbehaving logic and make it scope only. I think that also helps to keep the parsed result (and settings, e.g. delimiter) as a single source of truth without having hardcoded regexes.

I think that actually did the trick! Because the slashes are very scope-specific, in hindsight, it would be better to implement that in the scope-case-rule itself. PR #574 is ready for review!

Fix from #574 is merged and released in 7.5.2

Thank you guys, works well for me

Was this page helpful?
0 / 5 - 0 ratings