Allow skipping targets based on text they contain by oldergod · Pull Request #1749 · diffplug/spotless

oldergod · 2023-07-08T10:40:08Z

Follow-up of #1738

Add an field to FormatExtension (Gradle) to accept lists of text which if detected will get a Formatter to skip a file as a whole and will not apply anything.
I didn't touch the maven plugin.

oldergod · 2023-07-08T10:41:24Z

lib/src/main/java/com/diffplug/spotless/Formatter.java

 	private Charset encoding;
 	private Path rootDir;
 	private List<FormatterStep> steps;
+	private List<String> skipIfContentContains;


I didn't add it as a step because every steps would then be repeating the same logic. I thought it best to filter files before thinking of computing them or not.

The approach you took is a bit faster, but it has some downsides

more code (we already have a mechanism for skipping based on content, now we have two mechanisms)

functionality spread (formatter was as simple as possible to operate the FormatterStep, now it has one more mechanism that it needs to have)

This is a useful feature to have, but I think it is better to implement it using FormatterStep.filterByContentPattern. Making the fast part of a program a little faster doesn't matter very much.

Thank you for the feedback. I removed most of the code and left the API in the Gradle extension + the tests.
I don't see how to use FilterByContentPatternFormatterStep so I took it out. This one is opting-in based on a pattern, and I would wanna do the reverse. I don't know how to do that from the existing class.
If this was sorted out, I would probably then, when setting the task, wrap all steps with a filtering one.
Let me know if you have better ideas on how to reuse FilterByContentPatternFormatterStep

I would

add a field boolean formatIfMatches to

spotless/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java

Lines 25 to 29 in d245b60

final class FilterByContentPatternFormatterStep extends DelegateFormatterStep {

final Pattern contentPattern;

FilterByContentPatternFormatterStep(FormatterStep delegateStep, String contentPattern) {

super(delegateStep);

then change this to if (matcher.find() == formatIfMatches)

spotless/lib/src/main/java/com/diffplug/spotless/FilterByContentPatternFormatterStep.java

Lines 25 to 29 in d245b60

final class FilterByContentPatternFormatterStep extends DelegateFormatterStep {

final Pattern contentPattern;

FilterByContentPatternFormatterStep(FormatterStep delegateStep, String contentPattern) {

super(delegateStep);

Now you can add a boolean to this method

spotless/lib/src/main/java/com/diffplug/spotless/FormatterStep.java

Lines 56 to 58 in d245b60

public default FormatterStep filterByContentPattern(String contentPattern) {

return new FilterByContentPatternFormatterStep(this, contentPattern);

}

Now, in FormatExtension, save the excludeContent, and apply it to each step at the end with steps.replaceAll

spotless/plugin-gradle/src/main/java/com/diffplug/gradle/spotless/FormatExtension.java

Lines 883 to 892 in d245b60

List<FormatterStep> steps;

if (togglePair != null) {

steps = new ArrayList<>(this.steps.size() + 2);

steps.add(togglePair.in());

steps.addAll(this.steps);

steps.add(togglePair.out());

} else {

steps = this.steps;

}

task.setSteps(steps);

Updated the code @nedtwigg with passing tests.

oldergod · 2023-07-08T10:42:17Z

lib/src/main/java/com/diffplug/spotless/Formatter.java

+		for (String skipIfContentContain : skipIfContentContains) {
+			if (unix.contains(skipIfContentContain))
+				return unix;
+		}


The logic might be executed twice across computeLineEndings and compute but they're both public so to guarantee skipIfContentContains are respected, I added it to both.

nedtwigg

Formatter should not change, use FormatterStep.filterByContentPattern.

oldergod · 2023-07-13T08:24:16Z

lib/src/main/java/com/diffplug/spotless/FormatterStep.java

 	public default FormatterStep filterByContentPattern(String contentPattern) {
-		return new FilterByContentPatternFormatterStep(this, contentPattern);
+		return filterByContentPattern(contentPattern, true);
+	}


I can update this one directly if you don't mind having an API change.

nedtwigg · 2023-07-14T17:22:42Z

Because I'm not able to push to this PR, I opened a new PR (#1755) where I added a few more commits. Thanks for a great PR, it will be merged shortly.

oldergod · 2023-07-14T18:16:58Z

Thank you

nedtwigg · 2023-07-17T22:55:43Z

Released in plugin-gradle 6.20.0.

…lugin to v2.40.0 (mulk/quarkus-googlecloud-jsonlogging!20) This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.39.0` -> `2.40.0` | --- ### Release Notes <details> <summary>diffplug/spotless</summary> ### [`v2.40.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#2400---2023-07-17) ##### Added - Added support for Protobuf formatting based on [Buf](https://buf.build/). ([#1208](diffplug/spotless#1208)) - `enum OnMatch { INCLUDE, EXCLUDE }` so that `FormatterStep.filterByContent` can not only include based on the pattern but also exclude. ([#1749](diffplug/spotless#1749)) ##### Fixed - Update documented default `semanticSort` to `false`. ([#1728](diffplug/spotless#1728)) ##### Changes - Bump default `cleanthat` version to latest `2.13` -> `2.17`. ([#1734](diffplug/spotless#1734)) - Bump default `ktlint` version to latest `0.49.1` -> `0.50.0`. ([#1741](diffplug/spotless#1741)) - Dropped support for `ktlint 0.47.x` following our policy of supporting two breaking changes at a time. - Dropped support for deprecated `useExperimental` parameter in favor of the `ktlint_experimental` property. </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

This MR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [flow-bin](https://github.com/flowtype/flow-bin) ([changelog](https://github.com/facebook/flow/blob/master/Changelog.md)) | devDependencies | minor | [`^0.216.0` -> `^0.217.0`](https://renovatebot.com/diffs/npm/flow-bin/0.216.1/0.217.2) | | [org.liquibase:liquibase-maven-plugin](http://www.liquibase.org/liquibase-maven-plugin) ([source](https://github.com/liquibase/liquibase)) | build | minor | `4.23.2` -> `4.24.0` | | [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.39.0` -> `2.40.0` | | [io.quarkus:quarkus-maven-plugin](https://github.com/quarkusio/quarkus) | build | patch | `3.4.1` -> `3.4.2` | | [io.quarkus:quarkus-universe-bom](https://github.com/quarkusio/quarkus-platform) | import | patch | `3.4.1` -> `3.4.2` | --- ### Release Notes <details> <summary>flowtype/flow-bin</summary> ### [`v0.217.2`](flow/flow-bin@15ccd14...dc93913) [Compare Source](flow/flow-bin@15ccd14...dc93913) ### [`v0.217.1`](flow/flow-bin@6af43b3...15ccd14) [Compare Source](flow/flow-bin@6af43b3...15ccd14) ### [`v0.217.0`](flow/flow-bin@f96ca32...6af43b3) [Compare Source](flow/flow-bin@f96ca32...6af43b3) </details> <details> <summary>liquibase/liquibase</summary> ### [`v4.24.0`](https://github.com/liquibase/liquibase/blob/HEAD/changelog.txt#Liquibase-4240-is-a-major-release) [Compare Source](liquibase/liquibase@v4.23.2...v4.24.0) </details> <details> <summary>diffplug/spotless</summary> ### [`v2.40.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#2400---2023-07-17) ##### Added - Added support for Protobuf formatting based on [Buf](https://buf.build/). ([#1208](diffplug/spotless#1208)) - `enum OnMatch { INCLUDE, EXCLUDE }` so that `FormatterStep.filterByContent` can not only include based on the pattern but also exclude. ([#1749](diffplug/spotless#1749)) ##### Fixed - Update documented default `semanticSort` to `false`. ([#1728](diffplug/spotless#1728)) ##### Changes - Bump default `cleanthat` version to latest `2.13` -> `2.17`. ([#1734](diffplug/spotless#1734)) - Bump default `ktlint` version to latest `0.49.1` -> `0.50.0`. ([#1741](diffplug/spotless#1741)) - Dropped support for `ktlint 0.47.x` following our policy of supporting two breaking changes at a time. - Dropped support for deprecated `useExperimental` parameter in favor of the `ktlint_experimental` property. </details> <details> <summary>quarkusio/quarkus</summary> ### [`v3.4.2`](quarkusio/quarkus@3.4.1...3.4.2) [Compare Source](quarkusio/quarkus@3.4.1...3.4.2) </details> <details> <summary>quarkusio/quarkus-platform</summary> ### [`v3.4.2`](quarkusio/quarkus-platform@3.4.1...3.4.2) [Compare Source](quarkusio/quarkus-platform@3.4.1...3.4.2) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This MR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

oldergod commented Jul 8, 2023

View reviewed changes

oldergod changed the title ~~Bquenaudon.2023 07 08.filterfileconsuming~~ Allow skipping targets based on text they contain Jul 8, 2023

oldergod mentioned this pull request Jul 8, 2023

Introduce Spotless for the whole repo Kotlin & Java square/wire#2528

Merged

nedtwigg requested changes Jul 12, 2023

View reviewed changes

Add targetExcludeIfContentContains API to the Gradle plugin

55c2a6b

oldergod commented Jul 13, 2023

View reviewed changes

Implement excludingByContent for Gradle extensions

6459aa8

nedtwigg mentioned this pull request Jul 14, 2023

Skip on content contains (append to #1749) #1755

Merged

nedtwigg closed this Jul 14, 2023

nedtwigg added a commit that referenced this pull request Jul 14, 2023

Skip on content contains (#1755 amends #1749 fixes #1738)

7ebdc6a

oldergod deleted the bquenaudon.2023-07-08.filterfileconsuming branch July 14, 2023 18:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow skipping targets based on text they contain#1749

Allow skipping targets based on text they contain#1749
oldergod wants to merge 2 commits intodiffplug:mainfrom
sandboxing:bquenaudon.2023-07-08.filterfileconsuming

oldergod commented Jul 8, 2023

Uh oh!

oldergod Jul 8, 2023

Uh oh!

nedtwigg Jul 12, 2023

Uh oh!

oldergod Jul 12, 2023

Uh oh!

nedtwigg Jul 12, 2023

Uh oh!

oldergod Jul 13, 2023

Uh oh!

oldergod Jul 8, 2023

Uh oh!

nedtwigg left a comment

Uh oh!

oldergod Jul 13, 2023

Uh oh!

nedtwigg commented Jul 14, 2023

Uh oh!

oldergod commented Jul 14, 2023

Uh oh!

nedtwigg commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	final class FilterByContentPatternFormatterStep extends DelegateFormatterStep {
	final Pattern contentPattern;

	FilterByContentPatternFormatterStep(FormatterStep delegateStep, String contentPattern) {
	super(delegateStep);

	public default FormatterStep filterByContentPattern(String contentPattern) {
	return new FilterByContentPatternFormatterStep(this, contentPattern);
	}

	List<FormatterStep> steps;
	if (togglePair != null) {
	steps = new ArrayList<>(this.steps.size() + 2);
	steps.add(togglePair.in());
	steps.addAll(this.steps);
	steps.add(togglePair.out());
	} else {
	steps = this.steps;
	}
	task.setSteps(steps);

Conversation

oldergod commented Jul 8, 2023

Uh oh!

oldergod Jul 8, 2023

Choose a reason for hiding this comment

Uh oh!

nedtwigg Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

oldergod Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

nedtwigg Jul 12, 2023

Choose a reason for hiding this comment

Uh oh!

oldergod Jul 13, 2023

Choose a reason for hiding this comment

Uh oh!

oldergod Jul 8, 2023

Choose a reason for hiding this comment

Uh oh!

nedtwigg left a comment

Choose a reason for hiding this comment

Uh oh!

oldergod Jul 13, 2023

Choose a reason for hiding this comment

Uh oh!

nedtwigg commented Jul 14, 2023

Uh oh!

oldergod commented Jul 14, 2023

Uh oh!

nedtwigg commented Jul 17, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants