Skip to content

Commit

Permalink
Merge PR #128 from nblockchain/wip/fixBugsReportedInCommitMsg
Browse files Browse the repository at this point in the history
FileConventions,commitlint: partial fix for [1].

[1] #120
  • Loading branch information
knocte committed Jul 19, 2023
2 parents 138844d + 6cb6d10 commit 4ece0b2
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 80 deletions.
20 changes: 20 additions & 0 deletions commitlint/plugins.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" +
Expand Down
18 changes: 15 additions & 3 deletions commitlint/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -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);
Expand All @@ -455,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;
}
Expand Down
78 changes: 1 addition & 77 deletions src/FileConventions.Test/FileConventions.Test.fs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module FileConventions.Test
module FileConventions.Test.RestOfTests

open System
open System.IO
Expand Down Expand Up @@ -273,82 +273,6 @@ let HasBinaryContentTest3() =
Assert.That(HasBinaryContent fileInfo, Is.EqualTo false)


[<Test>]
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)

[<Test>]
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)

[<Test>]
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)


[<Test>]
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)

[<Test>]

let DetectInconsistentVersionsInGitHubCIWorkflow1() =
Expand Down
1 change: 1 addition & 0 deletions src/FileConventions.Test/FileConventions.Test.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
</PropertyGroup>

<ItemGroup>
<Compile Include="WrapTextTests.fs" />
<Compile Include="FileConventions.Test.fs" />
<Compile Include="Program.fs" />
</ItemGroup>
Expand Down
109 changes: 109 additions & 0 deletions src/FileConventions.Test/WrapTextTests.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
module FileConventions.Test.WrapTextTests

open System

open NUnit.Framework

open FileConventions

[<Test>]
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)

[<Test>]
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)

[<Test>]
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)


[<Test>]
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)

[<Test>]
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)
5 changes: 5 additions & 0 deletions src/FileConventions/Library.fs
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,10 @@ let private WrapParagraph (text: string) (maxCharsPerLine: int) : string =
(wrappedText: string)
(remainingWords: List<Text>)
: string =

let isColonBreak (currentLine: string) (textAfter: Text) =
currentLine.EndsWith ":" && Char.IsUpper textAfter.Text.[0]

match remainingWords with
| [] -> (wrappedText + currentLine).Trim()
| word :: rest ->
Expand All @@ -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
| _,
Expand Down

0 comments on commit 4ece0b2

Please sign in to comment.