cz-conventional-changelog icon indicating copy to clipboard operation
cz-conventional-changelog copied to clipboard

Make max line width configurable

Open jviotti opened this issue 8 years ago • 19 comments

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.

jviotti avatar Jun 20 '16 15:06 jviotti

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.

floriangosse avatar Aug 15 '16 11:08 floriangosse

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.

huy-nguyen avatar Apr 28 '17 03:04 huy-nguyen

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

travi avatar Apr 28 '17 03:04 travi

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.

huy-nguyen avatar Apr 28 '17 14:04 huy-nguyen

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

LinusU avatar Apr 28 '17 14:04 LinusU

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

huy-nguyen avatar Apr 28 '17 14:04 huy-nguyen

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.

jimthedev avatar Apr 28 '17 15:04 jimthedev

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.

lwouis avatar May 24 '18 12:05 lwouis

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 avatar May 24 '18 15:05 travi

also, i still think the best option is for the prompt to give some feedback while typing the message, as suggested in https://github.com/commitizen/cz-cli/issues/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).

travi avatar May 24 '18 16:05 travi

Created PR (https://github.com/commitizen/cz-conventional-changelog/pull/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.

yinzara avatar Jan 11 '19 00:01 yinzara

@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.

travi avatar Feb 04 '19 21:02 travi

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 avatar Feb 06 '19 22:02 yinzara

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 avatar Feb 06 '19 22:02 yinzara

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 avatar Feb 06 '19 22:02 yinzara

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 avatar Feb 06 '19 23:02 yinzara

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

yinzara avatar Feb 06 '19 23:02 yinzara

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.

martinmcwhorter avatar Jan 03 '20 13:01 martinmcwhorter

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

zeorin avatar Jan 06 '20 09:01 zeorin

Closing stale issues

jviotti avatar Aug 15 '22 21:08 jviotti