Skip to content

commentChar default should use git config core.commentchar #3190

Closed
@jscheid

Description

@jscheid

Expected Behavior

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

Current Behavior

Default setting for commentchar is #.

Affected packages

  • cli
    core
    prompt
    config-angular

Context

Hi, thanks for this tool. The commentChar setting appears to default to # if not overridden explicitly:

// Strip comments if reading from `.git/COMMIT_EDIT_MSG` using the
// commentChar from the parser preset falling back to a `#` if that is not
// set
if (flags.edit && typeof opts.parserOpts.commentChar !== 'string') {
opts.parserOpts.commentChar = '#';
}

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.

Activity

escapedcat

escapedcat commented on May 22, 2022

@escapedcat
Member

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

jscheid

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor
escapedcat

escapedcat commented on May 22, 2022

@escapedcat
Member

Can I remove this test?

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

jscheid

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor

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

escapedcat

escapedcat commented on May 22, 2022

@escapedcat
Member

Yeah, makes sense

jscheid

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor

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

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor

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

escapedcat

escapedcat commented on May 22, 2022

@escapedcat
Member

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 :)

jscheid

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor

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

jscheid commented on May 22, 2022

@jscheid
ContributorAuthor

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

6 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @jscheid@escapedcat

        Issue actions

          commentChar default should use git config `core.commentchar` · Issue #3190 · conventional-changelog/commitlint