Add regex for line comparison #243
Conversation
This reverts commit 8c6c33b.
|
Also converts a inline header style's name to lower case, otherwise the comparison (and the example in the docs) fails. |
* In maven/xml we need xml:space="preserve" to prevent trimming of text values. * The XML attribute is multiline, not isMultiline. * inline styles are only available in version 4.2 - we should note this, as the suggested version is 4.1. * the inline example does only work, if the inline style name is lowercase. code fixed in previous PR.
|
Hello, |
mathieucarbou
added
in:core
|
Changing the header definition did not work, because the detection of header lines was orginally done by comparing lines with startsWith(beforeEachLine.rtrim()) By introducing a (optional) RegEx for the header lines, this and most other use cases can be covered. |
docs/index.md
Outdated
| <afterEachLine></afterEachLine> | ||
| <!--skipLine></skipLine--> | ||
| <firstLineDetectionPattern>(\s|\t)*/\*.*$</firstLineDetectionPattern> | ||
| <lastLineDetectionPattern>.*\*/(\s|\t)*$</lastLineDetectionPattern> | ||
| <allowBlankLines>false</allowBlankLines> | ||
| <isMultiline>true</isMultiline> | ||
| <multiline>true</multiline> |
There was a problem hiding this comment.
Please remove any change regarding multiline: I've correctly fixed in #247
| @@ -34,6 +36,7 @@ | |||
|
|
|||
| private Pattern skipLinePattern; | |||
| private Pattern firstLineDetectionPattern; | |||
| private Pattern otherLineDetectionPattern; | |||
There was a problem hiding this comment.
I would prefer that the name is called middleLineDetectionPattern
There was a problem hiding this comment.
Except it is for all lines except the first line - the last line must also pass this pattern (or the beforeEachLine test).
Finding the best name is often the most difficult task ;-)
There was a problem hiding this comment.
the last line must also pass this pattern
Why ? There is a pattern for the last line.
There was a problem hiding this comment.
But the code still checks beforeEachLine. I didn't want to change the logic and the RegEx happens when the startsWith(beforeEachLine) check happens.
| @@ -209,6 +257,7 @@ public void validate() { | |||
| check("endLine", this.endLine); | |||
| check("afterEachLine", this.afterEachLine); | |||
| check("firstLineDetectionPattern", this.firstLineDetectionPattern); | |||
| // other line detection pattern can be null | |||
There was a problem hiding this comment.
no need for this comment: it's like beforeEachLine ;-)
| final HeaderDefinition headerDefinition = new HeaderDefinition( | ||
| "javadoc2_style", "/**", " ** ", " **/", "", null, "^\\s*/\\*.*$", "^\\s*\\*.*$", "^.*\\*/\\s*$", false, true, false); | ||
| return new HeaderParser(new FileContent(new File(fileName), System.getProperty("file.encoding")), headerDefinition, new String[]{"copyright"}); | ||
| } |
There was a problem hiding this comment.
You would need also to have a more complete test (like CompleteMojoTest does) to also test the other plugin actions, like really testing the issue you want to fix.
| this(type); | ||
| this.firstLine = firstLine; | ||
| this.beforeEachLine = beforeEachLine; | ||
| this.endLine = endLine; | ||
| this.afterEachLine = afterEachLine; | ||
| this.skipLinePattern = compile(skipLinePattern); | ||
| this.firstLineDetectionPattern = compile(firstLineDetectionPattern); | ||
| this.otherLineDetectionPattern = compile(otherLineDetectionPattern); |
There was a problem hiding this comment.
you would need to update setPropertyFromString(...) too for xml header definition.
mathieucarbou
added
is:bug
This solves #242 by adding a new optional regex to compare the lines other than the first for multiline headers.