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 targetExclude to plugin-gradle README #324

Closed
netvl opened this issue Dec 20, 2018 · 8 comments
Closed

Add targetExclude to plugin-gradle README #324

netvl opened this issue Dec 20, 2018 · 8 comments

Comments

@netvl
Copy link

netvl commented Dec 20, 2018

My project (using Gradle) has a task which generates some Scala sources, and I add generated source directories to the scala source set:

        project.extensions.findByType<SourceSetContainer>()?.invoke {
            "main" {
                withConvention(ScalaSourceSet::class) {
                    scala {
                        srcDir(project.generatedScalaDirectory)
                    }
                }
            }
        }

        val Project.generatedScalaDirectory: Provider<Directory>
            get() = layout.buildDirectory.dir("generated-scala")

I don't really care about generated sources formatting, because they are generated upon each compilation to the build directory, and are not committed to the repository. But as I've just discovered, spotless check (with its scalafmt support) fails on these generated sources.

Since I don't really want these sources to be checked, I would prefer for there to be a way to set up and exclude clause, something like

spotless {
    scala {
        scalafmt().configFile("${project.rootDir}/.scalafmt.conf")
        exclude(project.generatedScalaDirectory)
    }
}

However, it appears that there is no way to do it. It seems that there are two options:

  1. Use ignoreErrorForPath to specify a path to the excluded file.

    This does not appear to work: I still get a failed build, regardless of what path I specify with this method. Its sources indicate that it expects a "relative path", but it is not really clear relative to where it should be; it does not seem to be the project directory or root project directory.

    Anyway, this approach does not scale because it seems that it requires all files to be specified, separately, and not just the enclosing directory, which does not really scale.

  2. Use the target method to explicitly specify only the src/main/scala and src/test/scala sources.

    This is suboptimal, because I want to exclude something. This means that I have to remember to add new directories, e.g. new source sets, say, integrationTest, or I risk silently losing style verification for them.

@nedtwigg
Copy link
Member

In the past, we've told people to use target for this, but you make a good case that it's common that users will want to exclude things without opting out of the automatic source detection. The main reason we have rejected this in the past is to keep the documentation simple. E.g. if I specify a target and an exclude, does one trump the other? What is the format for excludes, just folders or a full regex?

I'd be happy to merge a PR for this iff it is able to design the feature in a way that is easy to document and unambiguous.

@matthiasbalke
Copy link
Contributor

@nedtwigg But we already have an includes and exludes for the maven-plugin within the FormatterFactory.java and the logic which overrides the other in AbstractSpotlessMojo.java. Maybe it's time to catch up for the gradle plugin.

@nedtwigg
Copy link
Member

I'm maven-illiterate, so I leave it entirely up to PR authors to determine what canonical behavior there is. For gradle, I'm strongly opinionated that the built-in scriptability means that plugins are best designed when they have simple unambiguous APIs.

I'm still happy to merge a PR for this iff it is able to design the feature in a way that is easy to document and unambiguous. I'm sure that's achievable, but it's a friendly heads-up to PR authors that you'll need to build a v2 or v3 of the PR if we're able to find ambiguous cases.

nedtwigg added a commit that referenced this issue Feb 10, 2019
@nedtwigg nedtwigg mentioned this issue Feb 10, 2019
3 tasks
@nedtwigg
Copy link
Member

I added targetExclude in #353. I resolved the ambiguities in this way: the files that get formatted are target - targetExclude. When you call target, you wipe out any previous calls. Same with targetExclude.

Implementing this change is just a few lines of code, but it actually affects 2 other issues. For this reason I'm closing this issue out so that the discussion can continue in the PR. If the PR meets the needs of all the issues, then I'll merge and we'll be all set.

@nedtwigg
Copy link
Member

Published in 3.18.0.

@xinatcg
Copy link

xinatcg commented Dec 13, 2020

seem the doc is removed from the gradle readme, cannot see this doc on latest readme

    targetExclude 'src/main/codegen/**', 'src/test/codegen/**'
    // the files to be formatted = (target - targetExclude)
    // NOTE: if target or targetExclude is called multiple times, only the
    // last call is effective

@nedtwigg nedtwigg changed the title Better support for excluding files from processing Add targetExclude to plugin-gradle README Dec 15, 2020
@nedtwigg
Copy link
Member

Agreed this should be in the documentation, PR's welcome.

@nedtwigg nedtwigg reopened this Dec 15, 2020
@nedtwigg
Copy link
Member

It's in the docs now, not sure when it got fixed.

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

No branches or pull requests

4 participants