commitlint
commitlint copied to clipboard
How to handle long URLs in commit messages
Expected Behavior
I want to be able to add links with long URLs (over 100 characters) to the footer so that I'll be able to adjust the footer line length in my own rules, while still assuring that the commit message body will be formatted consistently.
Alternatively there should be another way to add long urls to commits without having to minify them, while still adhering to the commit message max-line-length rules.
Current Behavior
It is impossible to create a commit containing a long url (over 100 characters), with the default config without minifying it. I would prefer not to minify the URL as depending on the minification service the minified URL might not be online as long and I could potentially lose context information from the actual URL, which I can only find out by following the minfied one.
Putting a long URL in the body on a single line only works by adjusting the body-max-line-length
rule, reducing the usefulness of the configuration.
Example
docs(adl): commit message style convention
asserted by commitlint to [angular commit style]
https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum
according to the conventional commits convention.
Putting the URL as a markdown link in the footer does not work while adjusting footer-max-line-length
to Infinity
as it is recognized not as the footer, but as part of the body.
Example
docs(adl): commit message style convention
asserted by commitlint to [angular commit style] according to the
conventional commits convention.
[angular commit style]: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum
results in
husky > pre-commit (node v12.18.2)
β Preparing...
β Running tasks...
β Applying modifications...
β Cleaning up...
husky > commit-msg (node v12.18.2)
β§ input: docs(adl): commit message style convention
asserted by commitlint to [angular commit style] according to the
conventional-commits convention
[angular-commit-style]: https://github.com/conventional-changelog/commitlint/tree/master/@commitlint/config-conventional
β body's lines must not be longer than 100 characters [body-max-line-length]
β found 1 problems, 0 warnings
β Get help: https://github.com/conventional-changelog/commitlint/#what-is-commitlint
husky > commit-msg hook failed (add --no-verify to bypass)
This seems to be due to the conventional-commit-parser package only recognizing the footer by usage of either "BREAKING CHANGE:" or supplying some issue number link "#333".
I found this out by saving the above commit message to a test.txt
file and running the parser over it resulting in:
[
{
"type":"docs",
"scope":"adl",
"subject":"commit message style convention",
"merge":null,
"header":"docs(adl): commit message style convention",
"body":"asserted by commitlint to [angular commit style] according to the\n[conventional commits] convention.\n\n[angular commit style]: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum",
"footer":null,
"notes":[],
"references":[],
"mentions":[],
"revert":null
}
]
Affected packages
- [x] parse
- [x] config-angular
Possible Solution
A solution could be to supply the conventional-commits-parser with a regex for specific noteKeywords.
Another solution could be to introduce an option to somehow ignore single line urls in the body of commit messages from the testing of body-max-line-length
rule.
But my question would also be how others normally add URLs to commit messges. Do you just always minify them?
Hey @texhnolyze ,
hm, interesting usage. So far I never experienced this. Maybe because we rarely add URLs to commits. Or they haven't been over 100 chars. I agreed minifying URLs is kinda bad because as someone who reads the message I would like to see the real URL.
Personally I would just increase body-max-line-length
I guess.
I don't see the need to solve this in the main codebase for now. You could try a plugin maybe. Or maybe other will see this issue and jump in. You could also ask in our slack chat how other might handle this or get some feedback.
Here is my 2c for what it's worth.
Forgive me because I am not sure if [angular commit style]
is a static string or if that is contextual based on the current commit. If it's static and you are always going to use that then you could easily just pass in the former as a parser option.
module.exports = {
extends: ['@commitlint/config-conventional'], // My repo is using config-conventional you could use anything...
rules: {
'footer-max-line-length': [0, 'always'] // Make sure there is never a max-line-length by disabling the rule
},
parserPreset: {
parserOpts: {
noteKeywords: ['[angular commit style]'] // Create a custom keyword to distinguish the footer
}
}
};
In which case the following parses correctly:
docs(adl): commit message style convention
asserted by commitlint to [angular commit style] according to the conventional commits convention.
[angular commit style]: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum
If [angular commit style]
changes then you might need to create a keyword such that you can recognise the start of a footer. Such as link
, or links
:
module.exports = {
extends: ['@commitlint/config-conventional'], // My repo is using config-conventional you could use anything...
rules: {
'footer-max-line-length': [0, 'always'] // Make sure there is never a max-line-length by disabling the rule
},
parserPreset: {
parserOpts: {
noteKeywords: ['link:'] // Create a custom keyword to distinguish the footer
}
}
};
Then you could just append the word link
to your footer whenever you need to list a footer.
Then when you need a link you can simpy do the following:
docs(adl): commit message style convention
asserted by commitlint to [angular commit style] according to the conventional commits convention.
link: [angular commit style]: https://github.com/conventional-changelog/commitlint/tree/master/%40commitlint/config-conventional#type-enum
I would note though in both scenarios I have had to disable the footer line length because that line (without the link keyword appended) is longer than 100 characters anyway. So if you're going to disable line length somewhere I think it's a trade off between how much you want to control the body length vs how much you force a particular commit style for footers.
@iamscottcab Nice that actually helped a lot. I did not realize, that you are able to add parserOpts
in your configuration.
Yeah [angular commit message]
was just an example, which changes depending on the link text I want to use.
Thanks to your examples and looking at the actual implementation of the noteKeywords parser options I constructed this:
module.exports = {
extends: ['@commitlint/config-conventional'],
rules: { 'footer-max-line-length': [1, 'always', 100] },
parserPreset: { parserOpts: { noteKeywords: ['\\[.+\\]:'] } },
}
This is somewhat a workaround due to me realizing, that the parser actually constructs a regex with the list of noteKeywords
I pass it. It enables me however to add any link in the format [any text]:
to the footer, which is a pattern I do not expect anywhere within the body of a normal commit message.
I also opted to still get a warning for longer than 100 character footers so people will have a second look, when it is not necessary.
I'll close this as my issue is solved. Thanks guys!
IMHO this isn't really resolved, because noteKeywords
is undocumented AFAICS.
I think it would be nicer if it could just spot long URLs and understand that they need to be on a single line and therefore an exception to body-max-line-length
. The above workaround definitely seems like a case of the human having to uncomfortably adapt to the whims of the machine, whereas good tooling should work the other way around.
IMHO this isn't really resolved, because
noteKeywords
is undocumented AFAICS.
Happy for a PR here
What does noteKeywords
do exactly? Would need to know that before being able to write any docs.
Tried to find some info for this.
It's just being handed to the conventional-commits-parser
notekeywords are documented here:
https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-commits-parser#notekeywords
For posterity could we get a clearly documented summary solution here? The idea is to add noteKeywords? That is turn overrides the rules? My use case doesn't involve putting the link in the footer, does that change any of the above?
Absolutely. I would also like to know recommended approach here
The workaround does not work when one has more than one URL, e.g.:
chore: remove eslint-config-prettier
ESLint deprecated formatting rules so we do not need the plugin anymore
https://eslint.org/blog/2023/10/deprecating-formatting-rules
https://github.com/eslint/eslint/pull/17696
Signed-off-by: Sebastian Davids <[email protected]>
I would prefer if either lines with URLs would get ignored by default or if there is an option to opt-in.
And please reopen this issue.
Let's summarise what we know so far from the above.
Firstly it's clear that this is an issue for more than a small handful of people.
Unsatisfactory workarounds
We have at least three workarounds, but unfortunately they are all really poor:
-
A workaround based on moving URLs to the footer is far from satisfactory since even if it was documented, it requires users to waste time on configuration and then remember an obscure trick with footers every time they want to mention URLs in commit messages.
-
Minifying URLs harms readability and also places a brittle dependency on third-party URL redirection services to stay running and maintain all redirects forever.
-
Increasing
body-max-line-length
significantly weakens the value of commitlint, since it entirely prevents the use of a helpful linting rule just for the sake of solving an occasional corner case.
Based on the above and the relatively large number of :+1: on my previous comment, we need to find a solution which does the right thing immediately out of the box, rather than forcing users to waste time jumping through hoops.
So IMHO this definitely needs to be addressed within the main codebase.
A user-friendly solution
I can't see any reason why the tool shouldn't simply ignore lines with URLs by default, as @sdavids already suggested:
- It should instantly solve the problem for all users whilst not introducing any breaking behaviour.
- It should not require the user to configure anything or change their behaviour.
- It should not be a complicated change to the parser. I see that there is already a
skipEmptyLines()
function used for header parsing, so presumably it would be easy to do something similar for skipping URLs when parsing the body.
[EDIT based on below discussion: To clarify, the proposal is to ignore long lines with URLs by default, except for on the first line, since best practice requires keeping the first line as a concise summary of the commit, and putting a long URL on the first line violates this best practice.]
A good first step here would be for the maintainer(s) to state whether they would be open to a PR implementing this solution.
Hm, not sure. Isn't the length limit there to make sure a message can be displayed and read in all contexts like on github, in tig, in places where only the first ~70 characters are displayed and everything else is being hidden? If you have long URLs in the subject that kinda defeats the purpose? Maybe this will break the current workflow for a lot of users which do not look into this issue? Could this be an optional flag maybe?
@escapedcat we normally don't have a choice about the length of the URLs that we're including in the commit message, unless we apply the workaround @aspiers mentioned of minifying URLs. Since URLs shouldn't be line wrapped, there is simply no viable alternative to exceeding the 70 character limit that those tools expect. The question for me is how best to deal with the inevitability of exceeding 70 character lines, and I'd add my vote to simply ignoring lines containing URLs as previously suggested.
Currently I'm thinking this should be handled via a plugin if this is possible.
I see that this is an issue for some people but the majority of users will be confused if this behaviour would change. URLs shouldn't be added to the subject but to the body or footer I believe.
Hm, not sure. Isn't the length limit there to make sure a message can be displayed and read in all contexts like on github, in tig, in places where only the first ~70 characters are displayed and everything else is being hidden?
Not really. Firstly, I don't think GitHub will ever truncate lines and prevent you from reading whole lines. Secondly, tig
has :set wrap-lines = yes
which I have just tested and indeed does allow reading of full lines.
But anyway, if a git tool permanently hides git data from you, then that is certainly a bug with that tool which needs to be fixed. And IMHO it doesn't makes sense to cripple one tool in order to work around breakage in another tool - that path will lead to misery and despair ;-)
If you have long URLs in the subject that kinda defeats the purpose?
I don't understand this sentence, and I see from this and your more recent comment that maybe I accidentally caused a misunderstanding. I'm assuming that by "subject" you mean the first line of the commit message, right? I don't think it makes sense to have long URLs in the first line; in fact I think it's actively harmful. And from your next comment, it seems you agree :-) But just to make sure we're on the same page, here's my take on this:
The first line should always concisely summarise the commit, and a long URL would be in direct conflict with that.
Consequently the user-friendly solution I'm proposing would ignore long URL lines anywhere except for the first line. Apologies that I didn't explicitly state that before, but hopefully it's clear now.
Maybe this will break the current workflow for a lot of users which do not look into this issue?
I can't think of a single way it would break the current workflow. Please can you give an example?
Could this be an optional flag maybe?
I honestly think that it would be a mistake to not enable this behaviour by default, because a good tool does the most sensible thing out of the box, and for the reasons detailed in my last comment, the user-friendly solution I proposed seems to completely solve the problem whilst not introducing a single additional downside.
But given the choice between it being an option which is off by default, and not having it as an option at all, I'd gladly choose the former.
Currently I'm thinking this should be handled via a plugin if this is possible.
Again, IMHO that would be a mistake because it would impose extra burden on users to set up the plugin when there seems to be no good reason to enable it out of the box.
I see that this is an issue for some people but the majority of users will be confused if this behaviour would change?
Why? Please can you give an example of how they would be confused?
URLs shouldn't be added to the subject but to the body or footer I believe.
As per above I agree 100% with this! The fact that you are mentioning this suggests that I didn't write my proposed solution clearly enough before and as a result you may have misunderstood it. But hopefully it's clear now.
I also thought that we were talking about the body and footer.
@aspiers got it, thanks for clearing this up. At one point in time I felt like people talk about the subject, my fault, sorry.
With github and tig etc I meant the default view, like here. But since this is the subject this is not an issue.
In this case I guess we would be fine with ignoring URLs for body/footer. Still wondering if this would be a breaking change. wdyt?
Great, thanks! Glad we are aligned now :rocket:
I can't think of any reasons why it would be a breaking change, and I haven't seen anyone else suggest any yet either.
The only scenario I can think of is if someone preferred minified URLs to long URLs and wanted the linter to warn when minification was effectively required. However, as already mentioned above, minified URLs are known to be a bad idea, and this tool is already somewhat opinionated, so personally I don't see an issue here.
That said, if there's a real concern about backwards compatibility then this could easily be solved by adding an option to disable ignoring of long URLs in the body and footer. But even if this option was added, I strongly recommend making the default behaviour to ignore these long URLs, because in most cases, if not all, minification would not be best practice.
I can't think of any reasons why it would be a breaking change, and I haven't seen anyone else suggest any yet either.
I'm just afraid of a bigger number of users never looking at this and relying on current behaviour for whatever reason. You never know.
If your up for a PR and maybe for a follow-up PR (if needed) I'm glad to merge it.
Yeah, it's impossible to know for sure, but it feels like a pretty safe change to me since it would only affect lines containing long URLs in the body and footer, and nothing else.
To be extra safe and correct, we should require that a long URL line is only ignored when no other words appear on the same line, since if there were other words, then this would make the already long line unnecessarily longer. However I think it would be worth allowing whitespace and punctuation, so that Markdown formatting and similar would be accepted. For example the following commit message would be valid:
docs: add something cool blah blah
We're adding something cool to the docs, based on info
from the following pages:
- https://example.com/blah/blah/really/really/really/really/really/long/URL
- https://example.com/blah/blah/another/really/really/really/really/long/URL
Even if it turns out that there is some weird reason why users really want the old behaviour, we could make it optional in a subsequent release, and in the interim period, those users could simply pin their version to the one prior to this change. So even this worst case scenario should not cause any significant issues for anyone.
Side note: is it possible to have something like prettier prettify the body before committing? That way we ensure that our commit messages are within the 100 line-length restriction (not sure if this would fix the long URL problem).
It is giving us issues, see here.
@aspiers how do you feel about markdown links then? My original intention when I had this issue was to be able to link URLs within the commit message with markdown, as github renders these even in commit messages.
So either with inline links:
feat(subject): added feature
for more context look [here](https://google.com)
or references:
feat(subject): added feature
for more context look [here]
[here]: https://google.com
I personally find this cleaner, which is why I opted for the footer workaround. It is true, that this also leads to issues when linking multiple URLs, even with the footer workaround.
Do you think the following rules would make sense:
- lines can be longer than the max length, when they contain URLs under the following conditions
- matches regex
^[\s#-]*
followed by URL for single line URLS, but supporting formatting - matches inline markdown links regex
\[[\w- ]+\]
followed by URL in()
- matches markdown link refrences regex
^\[[\w- ]+\]:
followed by URL
- matches regex
I think that way would make the most sense personally, but I can see that this could be a breaking change. And of course when introducing a breaking change, one could argue to just ignore any line with a URL in it.
just ignore any line with a URL in it.
I would prefer just thatβevery body/footer line with an URL is ignored.
*
-
+
1.
2)
*
.
#.
Maybe even:
>
#
The CommonMark link syntax is more complicated than you think, e.g. *[foo*](url)
is allowed.
AciiDoc uses a different syntax.
Going down this route and would lead to "Why do you support their syntax but not mine π€" β¦
I'm not sure I like "ignore every line that contains a link" - there can be cases like:
I have a short link https://bit.ly/abc but this trailing text now exceeds the max line width - whatever should I do?
I tend to use markdown links when writing some commit messages. I have not checked if they render right in GitHub but this is what I typically do:
This is a very long line with a long [link](
https://example.com/some/path/that/is/very/long/and/will/exceed/the/allotted/body/width
) and some text after.
Update
Splitting the link to a new line does not render neatly in GitHub.
@texhnolyze I agree it's a good idea to look for a solution which supports the various formats of linking which Markdown supports.
That said, I also agree with @sdavids that only supporting Markdown formats and nothing else is problematic. So I'm inclined to agree that maybe the best solution is indeed to ignore every line that contains a URL.
However we first need to make sure we can address @FelixZY's concerns:
I'm not sure I like "ignore every line that contains a link" - there can be cases like:
I have a short link https://bit.ly/abc but this trailing text now exceeds the max line width - whatever should I do?
In this case surely the line can be wrapped:
I have a short link https://bit.ly/abc but this trailing
text no longer exceeds the max line width - wrapping is
what I should do.
After all, keeping lines short where possible is the whole point of the rule. If someone doesn't want to keep lines short, they can simply disable the rule.
I tend to use markdown links when writing some commit messages. I have not checked if they render right in GitHub
According to @texhnolyze's comment they do render right.
but this is what I typically do:
This is a very long line with a long [link]( https://example.com/some/path/that/is/very/long/and/will/exceed/the/allotted/body/width ) and some text after.
Update
Splitting the link to a new line does not render neatly in GitHub.
You would have multiple options in this case, for example:
This is a very long line with a long [link](https://example.com/some/path/that/is/very/long/and/will/exceed/the/allotted/body/width)
and some text after.
or
This is a very long line with a long link:
- https://example.com/some/path/that/is/very/long/and/will/exceed/the/allotted/body/width
and some text after.
or
This is a very long line with a long [link][] and some text after.
[link]: https://example.com/some/path/that/is/very/long/and/will/exceed/the/allotted/body/width
Summary
I currently believe that the best solution is to ignore every line that contains a URL.
After all, keeping lines short where possible is the whole point of the rule. If someone doesn't want to keep lines short, they can simply disable the rule.
I guess I'm happy as long as the rule triggers if the line could have been wrapped.
@FelixZY commented on August 15, 2024 10:12 PM:
I guess I'm happy as long as the rule triggers if the line could have been wrapped.
Nice idea. I just finished an implementation, and I will see if I can tweak it to do that before submitting a PR.
Hrm, but how can we detect whether the line could have been wrapped?
For example, if the URL is inside a nested Markdown list:
- first list item
- second list item
- some text: https://example.com/now/here/is/a/longish/URL
Let's assume that this line with the URL currently exceeds the maximum line length configured by the rule. How do we know whether it should have been wrapped? If we somehow knew that it's Markdown format, and we had a built-in Markdown parser, we would be able to figure out that it could be written as:
- first list item
- second list item
- some text:
https://example.com/now/here/is/a/longish/URL
and maybe that would now be under the maximum line length without changing how the nested list is rendered.
But there's no way of knowing that it's Markdown format, and as @sdavids already observed, it's certainly not realistic to attempt to implement parsers for every possible format which might be used.
So I don't think there's any good way of achieving your suggestion.
I think i meant it more like "if the line could have been wrapped after the url", meaning that you would only raise an error if the url is followed by a whitespace and non-punctuation characters.
So this would not be an error:
- first list item
- second list item
- some text: https://example.com/now/here/is/a/longish/URL
- some more [text](https://example.com/now/here/is/a/longish/URL)
But this would:
- first list item
- second list item
- some text: https://example.com/now/here/is/a/longish/URL and trailing text