Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to automatically exclude other project directories from target(...) #468

Closed
vlsi opened this issue Oct 3, 2019 · 5 comments
Labels

Comments

@vlsi
Copy link
Contributor

vlsi commented Oct 3, 2019

It is quite easy to configure allprojects { spotless {..., however, it results in duplicate processing of the same files.

allprojects { // <-- !!!
    spotless {
        format("markdown") {
            target("**/*.md")
            trimTrailingWhitespace()
        }
    }
}

It turns out target("**/*.md") affects sub-projects as well.

In case for Apache JMeter we have a bunch of modules, and it would be great if `"**/*.md" did not descend to subprojects.
For instance, the root project should validate its own files, then subprojects should validate their own files, and so on.

I might see someone might want a "flat rule" when the rule is declared at the root project and verifies all the files, however, it is not that convenient because in that case I can no longer invoke spotlessCheck to check all the files relevant for the current submodule.

@nedtwigg
Copy link
Member

nedtwigg commented Oct 3, 2019

Can you do target('*.md'), or target('src/**/*.md')? Anywhere else in gradle (or computers generally), **/* will recurse on all subfolders. DiffPlug does one semi-clever thing by automatically ignoring the build directory, .git directory, and .gradle directories. Seems very reasonable, but that non-obvious optimization caused problems for users, who purposefully meant to include their build directory.

Also, I know there are folks who did target '**/*.gradle' in the root project with the explicit goal of formatting all subproject gradle files.

If Spotless were to implement this change, how should it look? targetDontIncludeSubProjects('**/*.md')? And hope that people find it in the docs?

@vlsi
Copy link
Contributor Author

vlsi commented Oct 3, 2019

Can you do target('*.md')

I can not.

I want to configure spotless once for "all the markdown files" and be done with it.
If I fine-tune to very specific names, then new markdown files might easily creep unnoticed.

Well. There's a question if spotless should be applied at sub-project level at all.
Technically speaking, applying Spotless plugin multiple times might increase project configuration time (and Spotless would have to create its dynamic configurations multiple times, and so on). So it is bad.

On the other hand, having "project-local" tasks for spotlessCheck / spotlessApply does help a lot when gw is used instead of gradlew
In other words, I can descend to a submodule. I can call gw style and it would verify files in this subfolder only. If I configure spotless at the root level, it would complain on the dirty files even in the case I'm not going to include them to the commit.

Anywhere else in gradle (or computers generally), */ will recurse on all subfolders

I'm afraid you are not quite right here.
PatternFilterable in Gradle (which is used in lots of places where path filtering is needed) excludes things like .svn, .git and so on.

See gradle/gradle#2986 and gradle/gradle#1348 (comment)

Also, I know there are folks who did target '**/*.gradle' in the root project with the explicit goal of formatting all subproject gradle files

That is true. Why don't they opt-in for "making this path to include sub-projects"?

Well, I'm even ok with "opt-in feature" that would exclude project directories of other projects. I can configure it just once and that is it.

If Spotless were to implement this change, how should it look?

target { // <-- well, target might be not the best name, however, it is already used in Spotless
    include("**/*.md") // default API of PatternFilterable
    exclude("**/*.md") // default API of PatternFilterable
    excludeProjectDirectories()
    excludeBuildDirectories()
    gitignore() // apply gitignore rules (!)
    ...
}

@nedtwigg
Copy link
Member

nedtwigg commented Oct 3, 2019

PatternFilterable in Gradle (which is used in lots of places where path filtering is needed) excludes

Maybe we're doing it wrong, but this is how we get PatternFilterable, and at least in the past it included .git, and apparently it still includes subprojects.

PatternFilterable userExact; // exactly the collection that the user specified
if (target instanceof String) {
userExact = getProject().fileTree(dir).include((String) target);
} else {
// target can only be a List<String> at this point
@SuppressWarnings("unchecked")
List<String> targetList = (List<String>) target;
userExact = getProject().fileTree(dir).include(targetList);
}

It seems like you have thought this through, and you raise good points. This is the entirety of our target specification. Our documentation is "give us a string, and we will pass it to project.fileTree".

If we're not following the conventions you expect, I'm open to a design and PR for a better API. I'm always hostile to breaking changes - the whole point of Spotless is that this stuff doesn't matter, just pick a default and stick with it. Once people have picked Spotless, they shouldn't have to muck with their spotless block.

The most important part of the design is the documentation. If you want a change, I would start by changing this: https://github.com/diffplug/spotless/tree/master/plugin-gradle#custom-rules

@nedtwigg
Copy link
Member

nedtwigg commented Oct 3, 2019

Maybe implementing targetExcludeSubprojects() would solve your problem? But if you have a more holistic fix I'm interested in that too.

@vlsi vlsi closed this as completed Nov 12, 2019
@vlsi
Copy link
Contributor Author

vlsi commented Nov 12, 2019

Migrated to autostyle/autostyle#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants