From d06966a63718689df4fa2556735de195e1ead301 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 17 Jul 2023 15:02:22 +0800 Subject: [PATCH 1/4] commitlint(plugins): add couple of comments To make the code more understandable. Also: a micro- optimization, as there's no need to get nextLine if code doesn't get inside the next block. --- commitlint/plugins.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/commitlint/plugins.ts b/commitlint/plugins.ts index f772d83ef..2ca9b0158 100644 --- a/commitlint/plugins.ts +++ b/commitlint/plugins.ts @@ -430,6 +430,8 @@ export abstract class Plugins { for (let paragraph of paragraphs) { let lines = paragraph.split(/\r?\n/); let inBigBlock = false; + + // NOTE: we don't iterate over the last line, on purpose for (let i = 0; i < lines.length - 1; i++) { let line = lines[i]; @@ -441,9 +443,10 @@ export abstract class Plugins { continue; } - let nextLine = lines[i + 1]; - if (line.length < paragraphLineMinLength) { + // this ref doesn't go out of bounds because we didn't iter on last line + let nextLine = lines[i + 1]; + let isUrl = Helpers.isValidUrl(line) || Helpers.isValidUrl(nextLine); From 933701843bb66e2c52e71dbf7bb601eccedb8def Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 17 Jul 2023 16:00:55 +0800 Subject: [PATCH 2/4] FileConventions.Test: separate tests to new file --- .../FileConventions.Test.fs | 78 +---------------- .../FileConventions.Test.fsproj | 1 + src/FileConventions.Test/WrapTextTests.fs | 83 +++++++++++++++++++ 3 files changed, 85 insertions(+), 77 deletions(-) create mode 100644 src/FileConventions.Test/WrapTextTests.fs diff --git a/src/FileConventions.Test/FileConventions.Test.fs b/src/FileConventions.Test/FileConventions.Test.fs index 15ba902ec..21c768dba 100644 --- a/src/FileConventions.Test/FileConventions.Test.fs +++ b/src/FileConventions.Test/FileConventions.Test.fs @@ -1,4 +1,4 @@ -module FileConventions.Test +module FileConventions.Test.RestOfTests open System open System.IO @@ -273,82 +273,6 @@ let HasBinaryContentTest3() = Assert.That(HasBinaryContent fileInfo, Is.EqualTo false) -[] -let WrapTextTest1() = - let characterCount = 64 - - let paragraph = - "This is a very very very very long line with more than 64 characters." - - let expectedResult = - "This is a very very very very long line with more than 64" - + Environment.NewLine - + "characters." - - Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) - -[] -let WrapTextTest2() = - let characterCount = 64 - - let paragraph = - "This is short line." - + Environment.NewLine - + "```" - + Environment.NewLine - + "This is a very very very very long line with more than 64 characters inside a code block." - + Environment.NewLine - + "```" - - let expectedResult = paragraph - - Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) - -[] -let WrapTextTest3() = - let characterCount = 64 - let tenDigits = "1234567890" - - let seventyChars = - tenDigits - + tenDigits - + tenDigits - + tenDigits - + tenDigits - + tenDigits - + tenDigits - - let paragraph = - "This is short line referring to [1]." - + Environment.NewLine - + "[1] someUrl://" - + seventyChars - - let expectedResult = paragraph - - Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) - - -[] -let WrapTextTest4() = - let characterCount = 64 - - let text = - "This is short line." - + Environment.NewLine - + Environment.NewLine - + "This is a very very very very very long line with more than 64 characters." - - let expectedResult = - "This is short line." - + Environment.NewLine - + Environment.NewLine - + "This is a very very very very very long line with more than 64" - + Environment.NewLine - + "characters." - - Assert.That(WrapText text characterCount, Is.EqualTo expectedResult) - [] let DetectInconsistentVersionsInGitHubCIWorkflow1() = diff --git a/src/FileConventions.Test/FileConventions.Test.fsproj b/src/FileConventions.Test/FileConventions.Test.fsproj index e6224691f..656568a1a 100644 --- a/src/FileConventions.Test/FileConventions.Test.fsproj +++ b/src/FileConventions.Test/FileConventions.Test.fsproj @@ -8,6 +8,7 @@ + diff --git a/src/FileConventions.Test/WrapTextTests.fs b/src/FileConventions.Test/WrapTextTests.fs new file mode 100644 index 000000000..0c1c9dc30 --- /dev/null +++ b/src/FileConventions.Test/WrapTextTests.fs @@ -0,0 +1,83 @@ +module FileConventions.Test.WrapTextTests + +open System + +open NUnit.Framework + +open FileConventions + +[] +let WrapTextTest1() = + let characterCount = 64 + + let paragraph = + "This is a very very very very long line with more than 64 characters." + + let expectedResult = + "This is a very very very very long line with more than 64" + + Environment.NewLine + + "characters." + + Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) + +[] +let WrapTextTest2() = + let characterCount = 64 + + let paragraph = + "This is short line." + + Environment.NewLine + + "```" + + Environment.NewLine + + "This is a very very very very long line with more than 64 characters inside a code block." + + Environment.NewLine + + "```" + + let expectedResult = paragraph + + Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) + +[] +let WrapTextTest3() = + let characterCount = 64 + let tenDigits = "1234567890" + + let seventyChars = + tenDigits + + tenDigits + + tenDigits + + tenDigits + + tenDigits + + tenDigits + + tenDigits + + let paragraph = + "This is short line referring to [1]." + + Environment.NewLine + + "[1] someUrl://" + + seventyChars + + let expectedResult = paragraph + + Assert.That(WrapText paragraph characterCount, Is.EqualTo expectedResult) + + +[] +let WrapTextTest4() = + let characterCount = 64 + + let text = + "This is short line." + + Environment.NewLine + + Environment.NewLine + + "This is a very very very very very long line with more than 64 characters." + + let expectedResult = + "This is short line." + + Environment.NewLine + + Environment.NewLine + + "This is a very very very very very long line with more than 64" + + Environment.NewLine + + "characters." + + Assert.That(WrapText text characterCount, Is.EqualTo expectedResult) From 948e27ee910d02d9a4ddba810f59dc5af35e06e4 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Mon, 17 Jul 2023 15:15:50 +0800 Subject: [PATCH 3/4] commitlint,FileConvention.Test: add failing tests The testcase comes from: https://github.com/nblockchain/conventions/issues/120#issuecomment-1637457272 --- commitlint/plugins.test.ts | 20 +++++++++++++++++ src/FileConventions.Test/WrapTextTests.fs | 26 +++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/commitlint/plugins.test.ts b/commitlint/plugins.test.ts index 7a2475248..f2e36b65b 100644 --- a/commitlint/plugins.test.ts +++ b/commitlint/plugins.test.ts @@ -410,6 +410,26 @@ test("body-paragraph-line-min-length6", () => { expect(bodyParagraphLineMinLength6.status).toBe(0); }); +test("body-paragraph-line-min-length7", () => { + let commitMsgThatSubceedsBodyMinLineLengthButIsLegit = + "Fixed bug (a title of less than 50 chars)" + + "\n\n" + + "These were the steps to reproduce:\n" + + "Do foo.\n" + + "\n" + + "Current results:\n" + + "Bar happens.\n" + + "\n" + + "Expected results:\n" + + "Baz happens."; + + let bodyParagraphLineMinLength7 = runCommitLintOnMsg( + commitMsgThatSubceedsBodyMinLineLengthButIsLegit + ); + + expect(bodyParagraphLineMinLength7.status).toBe(0); +}); + test("commit-hash-alone1", () => { let commitMsgWithCommitUrl = "foo: this is only a title" + diff --git a/src/FileConventions.Test/WrapTextTests.fs b/src/FileConventions.Test/WrapTextTests.fs index 0c1c9dc30..13d8431e4 100644 --- a/src/FileConventions.Test/WrapTextTests.fs +++ b/src/FileConventions.Test/WrapTextTests.fs @@ -81,3 +81,29 @@ let WrapTextTest4() = + "characters." Assert.That(WrapText text characterCount, Is.EqualTo expectedResult) + +[] +let WrapTextTest5() = + let characterCount = 64 + + let text = + "Fixed bug (a title of less than 50 chars)" + + Environment.NewLine + + Environment.NewLine + + "These were the steps to reproduce:" + + Environment.NewLine + + "Do foo." + + Environment.NewLine + + Environment.NewLine + + "Current results:" + + Environment.NewLine + + "Bar happens." + + Environment.NewLine + + Environment.NewLine + + "Expected results:" + + Environment.NewLine + + "Baz happens." + + let expectedResult = text + + Assert.That(WrapText text characterCount, Is.EqualTo expectedResult) From 6cb6d106623a1b30bc318eaee18fd51855642686 Mon Sep 17 00:00:00 2001 From: "Andres G. Aragoneses" Date: Wed, 19 Jul 2023 18:45:06 +0800 Subject: [PATCH 4/4] commitlint,FileConventions: make tests pass Partial fix for: https://github.com/nblockchain/conventions/issues/120 Co-authored-by: webwarrior --- commitlint/plugins.ts | 11 ++++++++++- src/FileConventions/Library.fs | 5 +++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/commitlint/plugins.ts b/commitlint/plugins.ts index 2ca9b0158..f5e294320 100644 --- a/commitlint/plugins.ts +++ b/commitlint/plugins.ts @@ -458,7 +458,16 @@ export abstract class Plugins { nextWordLength + line.length + 1 > paragraphLineMaxLength; - if (!isUrl && !lineIsFooterNote && !isNextWordTooLong) { + let isLastCharAColonBreak = + line[line.length - 1] === ":" && + nextLine[0].toUpperCase() == nextLine[0]; + + if ( + !isUrl && + !lineIsFooterNote && + !isNextWordTooLong && + !isLastCharAColonBreak + ) { offence = true; break; } diff --git a/src/FileConventions/Library.fs b/src/FileConventions/Library.fs index c5d36f8b1..5ef78b15d 100644 --- a/src/FileConventions/Library.fs +++ b/src/FileConventions/Library.fs @@ -203,6 +203,10 @@ let private WrapParagraph (text: string) (maxCharsPerLine: int) : string = (wrappedText: string) (remainingWords: List) : string = + + let isColonBreak (currentLine: string) (textAfter: Text) = + currentLine.EndsWith ":" && Char.IsUpper textAfter.Text.[0] + match remainingWords with | [] -> (wrappedText + currentLine).Trim() | word :: rest -> @@ -214,6 +218,7 @@ let private WrapParagraph (text: string) (maxCharsPerLine: int) : string = } when String.length currentLine + word.Text.Length + 1 <= maxCharsPerLine + && not(isColonBreak currentLine word) -> processWords (currentLine + " " + word.Text) wrappedText rest | _,