Toggle formatting inline with spotless:off and spotless:on#691
Merged
Toggle formatting inline with spotless:off and spotless:on#691
Conversation
This was referenced Sep 10, 2020
Member
Author
|
Released in |
jbduncan
reviewed
Sep 13, 2020
plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java
Show resolved
Hide resolved
plugin-maven/src/test/java/com/diffplug/spotless/maven/generic/ToggleOffOnTest.java
Show resolved
Hide resolved
plugin-maven/src/main/java/com/diffplug/spotless/maven/FormatterFactory.java
Show resolved
Hide resolved
testlib/src/test/java/com/diffplug/spotless/generic/PipeStepPairTest.java
Show resolved
Hide resolved
Member
Author
Member
Author
|
Hi @jbduncan, I implemented some of the comments in the "Inception" PR linked above, and anything that I didn't implement I explained why. I'm happy to discuss further, or you're welcome to push code to this second "Inception PR" which cleans up the PipeStep a bit, and also adds some new functionality. The Inception PR is a lot more questionable than this one, so I'm going to wait until I've got feedback from you or some other stakeholder that it's the right approach before merging. |
Member
|
@nedtwigg Just wanted to finally acknowledge your comments above, sorry it took so long! I'm happy with what's been done already, so I'm happy to bring this PR to a close close. :) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Sometimes there is a chunk of code which you have carefully handcrafted, and you would like to exclude just this one little part from getting clobbered by the autoformat. Some formatters have a way to do this, some don't, but who cares. It would be nice if you could scatter
spotless:offandspotless:oninline in your code, and have it just work, no matter which formatter you're using. This PR makes that possible.Changing the defaults
The one hiccup is that, because it works for all languages / formatters, it isn't context-aware. Meaning that it doesn't care if
spotless:offis in a comment, a string constant, or wherever. It just does a dumb search for pairs ofspotless:off/spotless:on, and saves whatever is between them from being formatted. You can change the terms to be whatever you want, e.g.fmt:off,fmt:on, or whatever, but using something longer and uncommon like "spotless" is safer than something short like "fmt". You can also set a full regex if you want to try to make it comment-specific, but that's probably harder than it seems at first.How it is implemented
It implements this under the hood with
PipeStepPair, which has anInstep and anOutstep. TheInstep is the very first step which gets applied,Outis the last step, and all the other formatting steps go in between them. TheInstep finds matching off/on pairs, and stores the text between them. The intermediate steps like google-java-format or whatever do their work, and at the end theOutstep finds the off/on pairs again, which may have moved. Then it replaces those pairs with whatever was stored by theInstep.Limitations
The intermediate steps can't remove an in/out pair. If they do, there's no way for spotless to put them back together. For example, if you have a step that converts your whole file to uppercase, that will turn
spotless:offtoSPOTLESS:OFF, which breaks the pair. This isn't a problem for the vast majority of steps. One notable exception, though, is the license header step. Because it replaces the whole header, any off/on pairs within the header will get clobbered, which will cause spotless to fail with an error.