Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Slashs and commas are not allowd in scope? #341

Closed
4 tasks
Stupidism opened this issue May 16, 2018 · 7 comments
Closed
4 tasks

Slashs and commas are not allowd in scope? #341

Stupidism opened this issue May 16, 2018 · 7 comments

Comments

@Stupidism
Copy link

Stupidism commented May 16, 2018

Expected Behavior

without slash:

➜  cra-rewired-starter git:(master) ✗ gc -m "feat(store-modules): add runtime"
husky > npm run -s precommit (node v8.11.1)

 ✔ Running tasks for {src,config}/**/*.{js,jsx,json}
 ↓ Running tasks for src/**/*.{css,less} [skipped]
   → No staged files match src/**/*.{css,less}
husky > npm run -s commitmsg (node v8.11.1)

⧗   input: feat(store-modules): add runtime
✔   found 0 problems, 0 warnings

Cli-test:

➜  cra-rewired-starter git:(master) ✗ echo "feat(components, component): subject" | ./node_modules/.bin/commitlint
⧗   input: feat(components, component): subject
✔   found 0 problems, 0 warnings

➜  cra-rewired-starter git:(master) ✗ echo "feat(components/component): subject" | ./node_modules/.bin/commitlint
⧗   input: feat(components/component): subject
✔   found 0 problems, 0 warnings

Current Behavior

Looks like there's an extra new-line before my commit message:

➜  cra-rewired-starter git:(master) ✗ gc -m "feat(store/modules): add runtime"
husky > npm run -s precommit (node v8.11.1)

 ✔ Running tasks for {src,config}/**/*.{js,jsx,json}
 ↓ Running tasks for src/**/*.{css,less} [skipped]
   → No staged files match src/**/*.{css,less}
husky > npm run -s commitmsg (node v8.11.1)

⧗   input:
feat(store/modules): add runtime

✖   message may not be empty [subject-empty]
✖   type may not be empty [type-empty]
✖   scope may not be empty [scope-empty]
✖   found 3 problems, 0 warnings

Affected packages

  • cli
  • core
  • prompt
  • config-angular

Possible Solution

Steps to Reproduce (for bugs)

https://github.com/Stupidism/cra-rewired-starter/tree/3a1c47cdcda5f585c529f78b246bc470ce64c526

commitlint.config.js ```js ```

Context

Your Environment

➜  cra-rewired-starter git:(master) ✗ node -v
v8.11.1
➜  cra-rewired-starter git:(master) npm -v
6.0.0
➜ cra-rewired-starter git:(master) ✗ grep 'version' node_modules/@commitlint/cli/package.json
    "type": "version",
  "version": "6.2.0",
Executable Version
commitlint --version VERSION
git --version VERSION
node --version VERSION
@marionebl
Copy link
Contributor

marionebl commented Jun 2, 2018

I suspect this is caused by the additional newline, not the / in commit scopes

The errors you see and the diverging behaviour if direct CLI / stdin usage are consistent with the newline showing up in the commitlint output of your failing example - the commit parser we use assigns special meaning to the first line of a commit.

I don't see any way how commitlint could cause this, any idea what might add the newline on your side?

@Stupidism
Copy link
Author

@marionebl Did you try my repo?

@aguacongas
Copy link

aguacongas commented Jun 13, 2018

same behavior with #
Do you have a workarround ?

@Stupidism
Copy link
Author

@aguacongas I could only use dot instead of hash or slash or comma

@robin-drexler
Copy link

Ran into the same issue and tried to dig into it a bit.

If I understood correctly, commitlint falls back to conventional-changelog-angular's presetOpts when none are provided.

The headerPatterns specified there would allow special chars to be used in scopes: /^(\w*)(?:\((.*)\))?: (.*)$/

However, presetOpts does not appear to be empty, because it already contains { commentChar: '#' }, hence the angular fallback is not used. Instead it seems that conventional-commits-parser default headerPattern is used, which does not allow special chars in scopes. /^(\w*)(?:\(([\w$.\-* ]*)\))?: (.*)$/

@marionebl
Copy link
Contributor

marionebl commented Sep 4, 2018

Thanks for reporting and the analysis, folks.

https://github.com/marionebl/commitlint/blob/1d79828427c19c72add82ff46a6c893c389cb4c7/%40commitlint/parse/src/index.js#L7-L10

The appropriate is to be more granular about defaulting to conventional-changelog-angular and fall back to the angular preset values for all keys (if missing in parserOpts), e.g.

const changelogOpts = await defaultChangelogOpts; 
parserOpts = merge(changelogOpts, parserOpts);

Want to lend a hand @robin-drexler?

@miketdonahue
Copy link

All of this "stuff" should make its way into the documentation as well. It is very confusing to figure out what options are available. Even if the docs just have links pointing to other repo options.

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

No branches or pull requests

5 participants