commitlint icon indicating copy to clipboard operation
commitlint copied to clipboard

commentChar default should use git config `core.commentchar`

Open jscheid opened this issue 2 years ago • 10 comments

Expected Behavior

Default setting for commentchar is the output of git config core.commentchar.

Current Behavior

Default setting for commentchar is #.

Affected packages

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

Context

Hi, thanks for this tool. The commentChar setting appears to default to # if not overridden explicitly: https://github.com/conventional-changelog/commitlint/blob/1ee518caca0cf758c8404554063d1ca42cacba8d/%40commitlint/cli/src/cli.ts#L224-L229

I think it would be better if it defaulted to the output of git config core.commentchar. I know I can override it in my config, but other people might have it set to something else (I have mine set globally in ~/.gitconfig) so not sure how I can make that work without being able to specify a global override.

This was brought up before in #2351.

jscheid avatar May 21 '22 12:05 jscheid

Thanks! Uhm, sounds like a valid improvement. Do you have time to give that a try?

escapedcat avatar May 22 '22 09:05 escapedcat

Sure, I can give it a go. Can I remove this test? https://github.com/conventional-changelog/commitlint/commit/5badf6dc08116ed3557e6c780e55764b4f07ca67#diff-c89c0216f62496bace8eebb7c6e1f9c11a6f1a1f55acbff74afb358439363b88

jscheid avatar May 22 '22 10:05 jscheid

Can I remove this test?

"Adjusting" sounds better, sure. I assume you could cover different examples with tests, right? Like i.s. # and $?

escapedcat avatar May 22 '22 11:05 escapedcat

I meant that the test will fail after the change because #1234 isn't an empty commit when commentChar is $.

jscheid avatar May 22 '22 11:05 jscheid

Yeah, makes sense

escapedcat avatar May 22 '22 11:05 escapedcat

Should these instructions be working? It runs (Found 0 errors) but doesn't seem to pick up file changes in cli.test.ts. I'm on MacOS

jscheid avatar May 22 '22 11:05 jscheid

Ah, yarn test works (but only after yarn build). Might want to update the instructions. All good.

jscheid avatar May 22 '22 11:05 jscheid

Looks like it only picks up changes in non-test files and does not run tests (anymore). The README might still reffer to the pre-TS setup. Happy for any improvements you're willing to add :)

escapedcat avatar May 22 '22 11:05 escapedcat

Actually, can you help me understand that test case ☝️

It still passes when I comment out the line where it sets core.commentChar with git so that's irrelevant.

typeof opts.parserOpts.commentChar !== 'string' is false with fixtures/comment-char, so the line opts.parserOpts.commentChar = '#'; should never be executed.. and opts.parserOpts.commentChar remain at $ which would mean #1234 isn't empty. But the test passes.

So... does that mean opts.parserOpts.commentChar is ignored? What exactly is this test testing?

jscheid avatar May 22 '22 11:05 jscheid

So... does that mean opts.parserOpts.commentChar is ignored? What exactly is this test testing?

I've worked it out, the test was broken because it checks to see if subject is empty but with a commit message like #1234 the subject is always empty, regardless of comment char, because there's no xyz: header. I've changed the tests to use body instead.

PR pushed: #3191

jscheid avatar May 22 '22 14:05 jscheid

Works great now, thanks again!

jscheid avatar Sep 21 '22 06:09 jscheid