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

Make max line width configurable #26

Closed
jviotti opened this issue Jun 20, 2016 · 20 comments
Closed

Make max line width configurable #26

jviotti opened this issue Jun 20, 2016 · 20 comments

Comments

@jviotti
Copy link

jviotti commented Jun 20, 2016

As far as I've seen, 72 characters is a well known default. Is there a reason why this adapter hardcodes 100? I'd be awesome if it was at least configurable.

@floriangosse
Copy link

The default in git of max line width is 50 for the subject and 72 chars for all other lines. It is described in How to Write a Git Commit Message.

I agree, it would be nice if we could make the values configurable.

@huy-nguyen
Copy link

I forked the repo and created a new npm package that allows you to configure the line length. It's name is cz-conventional-changelog-customizable.

@travi
Copy link
Contributor

travi commented Apr 28, 2017

rather than fragmenting things further, a PR to this project would be a great idea for this

@huy-nguyen
Copy link

I can make a PR if one of the maintainers chime in and tell me how they want the line width configuration to look like.

@LinusU
Copy link
Contributor

LinusU commented Apr 28, 2017

How does the config look in your fork, I'm open to any suggestions :)

@huy-nguyen
Copy link

It's just a json file that specifies what the max line widths for header and body are with fallback to 100 characters.

@jimthedev
Copy link
Member

I am open to the idea but I'd require placing it using the existing config loader (typically a key under package.json)
https://github.com/commitizen/cz-cli/blob/master/package.json#L23

Specifically I'd recommend namespacing it to the adapter.

config > commitizen > "cz-conventional-changelog" > maxLineWidthHeader

An adapter should then be passed any of the config options under its configuration path.

@lwouis
Copy link

lwouis commented May 24, 2018

I'm using commitizen together with commitlint on a project. Commitizen is setup with cz-conventional-changelog, and commitlint is setup with @commitlint/config-conventional.

It would be great if somehow the rules of what a conventional commit message would be shared between those projects. Line length at 72 for instance is an annoying one because commitizen will let you type a subject > 72 lines, but then the pre-commit hook will reject it immediately after.

@travi
Copy link
Contributor

travi commented May 24, 2018

the situation @lwouis describes is exactly the situation my team is in as well.

however, to make matters worse, we use husky to run npm test on the pre-commit hook. since git fires pre-commit before commit-msg, the feedback we get about the length is delayed, so it ends up being even more wasteful.

i agree that using a project's commitlint config, if present, would be ideal. although, it might be a little less than simple to handle configs that extend others, like mine.

@travi
Copy link
Contributor

travi commented May 24, 2018

also, i still think the best option is for the prompt to give some feedback while typing the message, as suggested in commitizen/cz-cli#178 so that the message isn't just truncated w/o the developer getting some sort of warning (which is why i do see value in the commitlint length check).

@yinzara
Copy link
Contributor

yinzara commented Jan 11, 2019

Created PR (#75).

It allows config through the "config.commitzen" package.json key (the same place you specify "path" for the commitizen adapter). You can now configure the maxHeaderWidth, maxLineWidth, defaultType, defaultScope, defaultBody, and defaultIssues though the original environment variables support will override the config.

I also fixed a few issues with standardization required by the Conventional Commit standard that will fail with @commitlint/config-conventional . The scope now is always lowercase (which is required). The subject will now start with a lowercase letter and any trailing dots are trimmed.

Regarding the length, the length allowed for the subject is now a calculated value based upon config.maxHeaderWidth - type.length - (scope ? scope.length + 2 : 0) + 2 // for the colon space
The number of allowed character is printed in the prompt message for the subject and as you type, the count of character shows in parens next to the input. When the length is equal to or less than the allowed amount, the string shows as green. Once you have gone over, it shows red. If you attempt to submit while its red, it will fail and ask you to reenter.

Regarding body wrapping, this is now configurable via the "maxLineWidth" parameter which will set the wrapping amount for both the body and footer (default set to 100 as it is now).

Honestly I believe the default should be 72 for both header and body but since its configurable, each project can decide on their own. You will need to set it to 72 to allow commitlint to always pass for the config-conventional.

@travi
Copy link
Contributor

travi commented Feb 4, 2019

@yinzara i've been meaning to try out your PR on my personal projects, but havent had a chance to yet. It sounds very close to what I would hope for, so I'm excited about it.

Since you mention commitlint and configuration, do you think it could be feasible for the default config to be pulled from what is built up in the commitlint config, when present? I think I would always want them to be configured consistently, so needing to configure commitizen separately feels like it would leave too easy of an opportunity for them to get out of sync.

Personally, i use my own shareable config for commitlint, so feel free to take a look at that in case my approach is different from yours in any way that would influence feasibility.

@yinzara
Copy link
Contributor

yinzara commented Feb 6, 2019

Your wish is.... well I'm happy to add things I also agree are awesome.

I've now added optional parsing of commitlint config itself (still can be overwritten by the config I added or the environment variable config). If commitlint is not present, it fails gracefully and continues to function normally. If it is, it defaults the value based on the commitlint config (no matter how complex your import is as it uses commitlint itself to get the config values).

I also added test cases which required a mocking framework (I used mock-require since it supported mocking default functions [sinon does not]).

The only config parameter supported is the commitlint rule "header-max-length" (i.e. the maxHeaderWidth config parameter)

@yinzara
Copy link
Contributor

yinzara commented Feb 6, 2019

Oh and I forgot, related to issue #43 . I have added additional questions that only appear if you add a breaking change or an issue reference but have no description. It defaults the value to "-" but prompts you to change it.

@yinzara
Copy link
Contributor

yinzara commented Feb 6, 2019

Note regarding versions of things. As we have always supported Node 4 for this project, I've attempted to keep Node 4 compatibility. The @commitlint/load module (which handles commitlint loading) only appeared in version 6.1.1 of commitlint which doesn't support Node 4 (it's node 6 and higher). The library already failed gracefully however the test cases were breaking. I added a semver checking and added separate test cases for any Node before 6 that test that it fails gracefully.

@yinzara
Copy link
Contributor

yinzara commented Feb 6, 2019

Btw I also removed the "console.log" statement that appears every time the CLI is run. Since it now handles line wrapping and heading max width automatically, I felt it wasn't required and it was making running the test cases look awful.

@yinzara
Copy link
Contributor

yinzara commented Feb 6, 2019

Btw if you run code coverage, you'll see there is now 100% coverage ;-)

@martinmcwhorter
Copy link

I am looking at creating a PR to bring even more configuration options from commitlint.

Before doing this -- I am wondering if this would be desirable in this project?

I don't want to fragment this project unnecessarily.

@zeorin
Copy link

zeorin commented Jan 6, 2020

I'm not a maintainer of this project, just a user, but I whole-heartedly welcome any contributions that aligns it with commitlint.

@jviotti
Copy link
Author

jviotti commented Aug 15, 2022

Closing stale issues

@jviotti jviotti closed this as completed Aug 15, 2022
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 a pull request may close this issue.

10 participants