Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If a markdown formatted link exceeds 80 characters, PHPCS throws a LineLengthSniff warning. Is this intended? If so, is there documentation on how to format a markdown README.md file to avoid this warning?
Comments
Comment #2
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedYes, I also have this problem, in the Rules module readme.md. I have used
--ignore=*.md
so that the file is not tested (this is for Travis builds where we need a 0 exit code from phpcs)If the link cannot be shortened then we could alter the sniff not to report this "error".
Comment #3
ApacheEx CreditAttribution: ApacheEx commentedI think it's related to #2867601: CodeSniffer testing all file extensions instead of the configured ones
Comment #4
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedHi ApacheEx,
Thanks for the link, I seem to recall I had also noticed that the extensions list is not adhered to. But the actual problem I am reporting is how to get round the fact that markdown links currently cannot be wrapped to a new line so they often extend past 80 and cause a standards failure. The .md file can be checked for other standards so excluding the file entirely is not what we want.
Jonathan
Comment #5
Dropa CreditAttribution: Dropa at Mediamaisteri Oy commentedCoder in fact should be little less forcing on the 80 character rule with markdown, there are two examples where it is not easy, if even possible to shrink lines.
or say you would like to document usage of some plugin
I would suggest coder to completely ignore line lengths in codeblocks and in links (at least those that has been described in own line)
Comment #6
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedYes the sniff probably does need some work. In the meantime, if you have a particular file in your module that fails on linelength you can do like Rules does, and add the following to the project's phpcs.xml.dist file
Comment #7
pfrenssenThis also happens when referring to internal links by the way so the standard detection that we are doing right now (check if the line includes "https") is not sufficient. Internal links are quite common in Markdown files so you can link documentation pages internally and have it published on different domains like drupal.org and github.
I think a good solution could be to do something similar as is done in markdownlint. Their line length rule will check if there are no spaces occurring after the 80 character limit.
We could do a check like: if the line extends beyond 80 characters and does not contain any whitespace after character #40 then we will not throw the warning.
Ref. https://github.com/markdownlint/markdownlint/blob/master/docs/RULES.md#m...
Comment #8
pfrenssen@Dropa it is a good suggestion to ignore line lengths in code blocks but I think it's best to handle that in a separate issue.
Comment #9
itamair CreditAttribution: itamair as a volunteer commentedthis is heavily affecting (in a really un-smart way) the Security Process Review of the D* Geofield Map project: https://www.drupal.org/project/projectapplications/issues/3071886#commen...
Please indicate a feasible way to bypass this impediment.
Comment #10
jonathan1055 CreditAttribution: jonathan1055 as a volunteer commentedHi itamair,
Have you tried the solution I suggested in #6 above ?
Comment #11
itamair CreditAttribution: itamair as a volunteer commentedthanks @jonathan1055 ... but your workaround seem not to work with the Pareview.sh process (and indeed didn't work for my pareview) as more clearly explained time ago:
https://www.drupal.org/project/pareviewsh/issues/2865650
Comment #12
pfrenssenHere is a proposed fix: https://github.com/pfrenssen/coder/pull/57
@itamair can you check if this solves the false positive for you?
Comment #13
itamair CreditAttribution: itamair as a volunteer commentedthanks @pfrenssen ... and sorry,
but since I applied the #6 workaround (and then also removed it) the phpcs coding standard is not reporting those Readme.md link "longer than 80 chars" warnings anymore.
I exactly don't know why ... but actually I cannot test your PR,
because I cannot recreate this, anymore.
I should also mention that upgraded to Drupal Coder 8.x-3.x (I was using 8.x-2.x) before
(somehow, thanks to a smart and more practical reviewer, the Geofield Map module finally passed the Security Policy Review: https://www.drupal.org/project/projectapplications/issues/3071886)
I hope it would be easier for you to reproduce the Readme.md link "longer than 80 chars" warning (in a test module) and check your PR fix ...
Comment #14
pfrenssenNo problem, I'm glad you managed to solve it!
I have a test in the PR using two types of links that were previously failing. These are now fixed. I was just curious if your case was also solved.
Comment #16
klausiThanks, that makes sense. Merged!
I don't think this is a critical issue since people can always silence this sniff in their config. Coding standard issues are also not application blockers in the security advisory coverage queue.
Comment #17
itamair CreditAttribution: itamair as a volunteer commentedyes @klausi ... I agree, and that should be the case, in both theory and practice.
BUT it seems that some (not so skilled) reviewers make your Security Advisory Review rely mostly on the automatic Pareview.sh outcome,
pretending you to fix all the single (even the less meaningful) warnings (and not errors) before moving forward with the real security review itself.
Upon my direct experience that is a critical issue that makes the Security Advisory Review process so bureaucratic and cumbersome (not to say a real pain in the xxx ...).
Happy I got rid of it ... and just concerned for new developers.