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

Command Line arguments should allow to exclude a file #6399

Closed
rnveach opened this Issue Jan 30, 2019 · 7 comments

Comments

Projects
None yet
2 participants
@rnveach
Copy link
Member

rnveach commented Jan 30, 2019

Ant allows to exclude a specific file.

<exclude name="**/InputAstTreeStringPrinter.java" />
<exclude name="**/InputSuppressionsStringPrinter.java"/>
<exclude name="**/InputMainIncorrectClass.java"/>
<exclude name="**/InputJavadocPropertiesGeneratorParseError.java"/>
<exclude name="**/InputMainFrameModelIncorrectClass.java"/>
<exclude name="**/InputBeforeExecutionExclusionFileFilterIncorrectClass.java"/>
<exclude name="**/InputJavaParser.java"/>

http://checkstyle.sourceforge.net/cmdline.html#Command_line_usage

-e, --exclude excludedDirectory - Directory to exclude from CheckStyle. The directory can be the full, absolute path, or relative to the current path. Multiple excludes are allowed.
-x, --exclude-regexp excludedDirectoryPattern - Directory pattern to exclude from CheckStyle. Multiple excludes are allowed.

Both options say to exclude directory, not a file, and the source code agrees with the documentation.

if (node.isDirectory()) {
if (!isDirectoryExcluded(node.getAbsolutePath(), patternsToExclude)) {

I want to specify a file, because I was checking some things out with running checkstyle_resources_checks.xml through the command line and I can't fully reproduce it in the command line because I can't tell it to exclude the files that cause an exception.

UPDATE:
exclusion by file is possible by extending of existing CLI argument
-e, --exclude excludedPath</code> - Directory/file to exclude from CheckStyle. The path can be the full, absolute path, or relative to the current path.

@romani romani added the approved label Jan 30, 2019

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Jan 30, 2019

@romani Should this be a new option or should we extend the existing option to include files?
If it should be a new option, what letter(s) should we use for it? ef and xf?

@romani

This comment has been minimized.

Copy link
Member

romani commented Jan 30, 2019

-e, --exclude excludedDirectory - Directory to exclude from CheckStyle. The directory can be the full, absolute path, or relative to the current path. Multiple excludes are allowed.
-x, --exclude-regexp excludedDirectoryPattern - Directory pattern to exclude from CheckStyle. Multiple excludes are allowed.

What if we extend this properties to exclude Directory or File ?

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Jan 30, 2019

I am fine with this, but it will be considered a breaking change even though this is Main and we don't announce it as breaking.

@romani

This comment has been minimized.

Copy link
Member

romani commented Jan 30, 2019

why it is breaking ?
all scripts that used such option will/should continue to work.

@rnveach

This comment has been minimized.

Copy link
Member Author

rnveach commented Jan 30, 2019

For the regular expression exclude, people may have written their expressions too relaxed or not specific enough and could match a file with this change and cause the file to be accidentally excluded. If it isn't considered breaking, people would still have to re-verify their scripts for no hidden problems.

@romani

This comment has been minimized.

Copy link
Member

romani commented Jan 30, 2019

Make sense, marked appropriately

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 17, 2019

romani added a commit that referenced this issue Feb 17, 2019

@romani romani added this to the 8.18 milestone Feb 17, 2019

@romani

This comment has been minimized.

Copy link
Member

romani commented Feb 17, 2019

fix is merged.

@romani romani changed the title Command Line can't exclude a file Command Line arguments should be allow to exclude a file Feb 17, 2019

@romani romani closed this Feb 17, 2019

@romani romani changed the title Command Line arguments should be allow to exclude a file Command Line arguments should allow to exclude a file Feb 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.