Skip to content

[MNG-8673] SourceRoot includes and excludes should be String #2232

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

Merged
merged 3 commits into from
Apr 13, 2025

Conversation

gnodet
Copy link
Contributor

@gnodet gnodet commented Apr 4, 2025

JIRA issue: MNG-8673

@@ -50,7 +49,7 @@ default Path directory() {
* The default implementation returns an empty list, which means to apply a language-dependent pattern.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed new block of comment:

    /**
     * {@return the list of patterns for the files to include}.
     * The path separator is {@code /} on all platforms, including Windows.
     * The prefix before the {@code :} character, if present, is the syntax.
     * If no syntax is specified, the default is a Maven-specific variation
     * of the {@code "glob"} pattern.
     *
     * <p>The default implementation returns an empty list, which means to apply a language-dependent pattern.
     * For example, for the Java language, the default pattern is {@code "*.java"}.</p>
     */

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if on windows we have a drive specified ? We need to differentiate:

C:/foo/**

So maybe rephrase as: If the pattern does not start with {@code regex:} or {@code glob:}, the default variation of the {@code glob} pattern will be used.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not tested how we can specify a Windows drive to java.nio.file.FileSystem.getPathMatcher(String). I was hopping that, since all paths are made relative to the base directory before to be tested for match, the Windows drive issue would be avoided.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to complete: "glob:" and "regex:" are the two syntaxes that all FileSystem shall support according NIO Javadoc, but a file system may support others syntaxes as well. It may be nice to avoid a formulation that restrict the possibilities to those two syntaxes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According Stackoverflow, Windows drives are restricted to 1 letter. So I suggest to replace "If the pattern does not start with regex: or glob:" by "The prefix before the : character, if present and longer than one character, is the syntax".

@desruisseaux
Copy link
Contributor

Added pull request #2236 as a follow-up of this one.

@gnodet gnodet merged commit c0b9106 into apache:master Apr 13, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants