Fix licenseHeader for Kotlin code starting with @file:...
#139
Conversation
|
Can you add a changelog entry under |
|
Done! |
There was a problem hiding this comment.
Hi @JLLeitschuh. I have a couple of comments, but otherwise this LGTM! :)
| @@ -25,18 +25,20 @@ | |||
| import com.diffplug.spotless.kotlin.KtLintStep; | |||
|
|
|||
| public class KotlinExtension extends FormatExtension { | |||
| // The delimiter has the '^' prepended to the regex where the pattern is compiled. | |||
There was a problem hiding this comment.
It took me a while to understand what this comment was saying, so I wonder if it would be clearer if it was worded something like this:
// '^' is prepended to the regex in LICENSE_HEADER_DELIMITER later in FormatExtension.licenseHeader(String, String)
| // Make sure the header gets added. | ||
| Assertions.assertThat(result).startsWith(HEADER); | ||
| // Make sure that the rest of the file is still there with nothing removed. | ||
| Assertions.assertThat(result).endsWith(original); |
There was a problem hiding this comment.
AFAICT, these two assertions don't guard against the case where unrelated text gets inserted in between the header and the rest of the original file contents, accidentally or otherwise.
Thus I wonder if it's worth testing that original directly follows HEADER.
(If so, one might be able to do this using String character indexes somehow, or alternatively checking that the file contains exactly as many lines as HEADER + original...)
There was a problem hiding this comment.
Interesting thought. I'll add something to assert that.
There was a problem hiding this comment.
I've expanded this test.
|
Should be all cleaned up now! |
|
Looks great to me now @JLLeitschuh. Thanks! |
|
I'll release as a bugfix later today. |
JLLeitschuh
deleted the
bug/JLL/kotlin_licenseHeader_file_annotation_delimiter
branch
Closes #136