If spotlessCheck fails, the error message could include a diff

[nedtwigg]

This would make it easier to understand what spotlessApply would do, if it were called. If anybody wants to contribute this functionality:

• FormatTask.formatCheck() is where the exception is exception is thrown right now.
• Formatter.isClean(File) is where the formatted and unformatted strings are both available. So that's where you'd get the things to diff.

[nedtwigg]

@jbduncan What do you think of the error messages as seen in DiffMessageFormatterTest?

They demonstrate what I was referring to with presenting line endings and whitespace "in-band" with the diffs.

[jbduncan]

@nedtwigg The error messages look very nice to me! I'm really impressed by the source code refactoring you did, it's made things look a lot neater now.

My only criticism with regards to the error messages is that especially large diffs that go over 50 lines do not always show how "-" lines differ from their corresponding "+" lines, as sometimes there's not enough space to show all or any of the "+" lines at all (as demonstrated in https://github.com/diffplug/spotless/blob/diff/src/test/java/com/diffplug/gradle/spotless/DiffMessageFormatterTest.java#L263-L311).

But I doubt there's a reasonable way of "fixing" this without rolling our own solution that would replace JGit's DiffFormatter class in https://github.com/diffplug/spotless/blob/diff/src/main/java/com/diffplug/gradle/spotless/DiffMessageFormatter.java#L151-L154, so I don't think it's worth pursuing for now.

[jbduncan]

I've also a couple of suggestions regarding the new tests you've written, which I raised in my comments at 15aeb77#commitcomment-19574416 and d5a770d#commitcomment-19574477.

Thought I'd link them here in case you missed them. :)

[jbduncan]

Overall, very good work! And I think once you've addressed my comments regarding the tests, the diff branch can be safely merged into master.

👍

[nedtwigg]

Shipped in 2.3.0.