Claudenw commented on code in PR #433:
URL: https://github.com/apache/creadur-rat/pull/433#discussion_r2072417944
##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/fileProcessors/AbstractFileProcessorBuilder.java:
##########
@@ -68,7 +68,7 @@ public abstract class AbstractFileProcessorBuilder {
/** The predicate that will return {@code false} for any comment line in
the file. */
protected final Predicate<String> commentFilter;
/** the collection of level builders */
- private final SortedMap<Integer, LevelBuilder> levelBuilders;
+ protected final SortedMap<Integer, LevelBuilder> levelBuilders;
Review Comment:
Let's not change this to protected.
Add a method something like
```
pubic LevelBuilder getLevelBuilder(int level) {
return levelBuilders.computeIfAbsent(level, k -> new LevelBuilder());
}
```
and call that in your GitIgnoreBuilder `initializeBuilder()` implementation.
I think this will be a more generically useful method.
##########
apache-rat-core/src/test/java/org/apache/rat/config/exclusion/fileProcessors/GitIgnoreBuilderTest.java:
##########
@@ -125,13 +135,50 @@ public void test_RAT_335() {
DocumentName candidate = DocumentName.builder()
.setName("/home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/dir1/file1.log")
.setBaseName("home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/").build();
- System.out.println("Decomposition for "+candidate);
+ System.out.println("Decomposition for " + candidate);
- assertThat(matcher.toString()).isEqualTo("matcherSet(or('included
dir1/.gitignore', 'included .gitignore'), or('excluded dir1/.gitignore',
**/.gitignore, 'excluded .gitignore'))");
+ assertThat(matcher.toString()).isEqualTo("matcherSet(or('included
.gitignore', 'included .gitignore'), or('excluded .gitignore', **/.gitignore,
'excluded .gitignore'))");
List<String> notMatching = Arrays.asList("README.txt", "dir1/dir1.md",
"dir2/dir2.txt", "dir3/file3.log", "dir1/file1.log");
- List<String> matching = Arrays.asList(".gitignore", "root.md",
"dir1/.gitignore", "dir1/dir1.txt", "dir2/dir2.md", "dir3/dir3.log");
+ List<String> matching = Arrays.asList(".gitignore", "root.md",
"dir1/.gitignore", "dir1/dir1.txt", "dir2/dir2.md", "dir3/dir3.log");
+
+ assertCorrect(matcherSets, documentName.getBaseDocumentName(),
matching, notMatching);
+ }
+
+ /**
+ * Test that exclusions from a global gitignore are also applied
+ *
+ * https://issues.apache.org/jira/browse/RAT-473
+ */
+ @Test
+ public void test_global_gitignore() {
+ URL globalGitIgnoreUrl =
GitIgnoreBuilderTest.class.getClassLoader().getResource("GitIgnoreBuilderTest/global-gitignore");
+ String globalGitIgnore = globalGitIgnoreUrl.getFile();
+
+ GitIgnoreBuilder underTest = new GitIgnoreBuilder() {
+ @Override
+ protected String globalGitIgnore() {
+ return globalGitIgnore;
+ }
+ };
+ URL url =
GitIgnoreBuilderTest.class.getClassLoader().getResource("GitIgnoreBuilderTest/src/");
+ File file = new File(url.getFile());
+
+ DocumentName documentName = DocumentName.builder(file).build();
+ List<MatcherSet> matcherSets = underTest.build(documentName);
+ DocumentNameMatcher matcher =
MatcherSet.merge(matcherSets).createMatcher();
+
+ DocumentName candidate = DocumentName.builder()
+
.setName("/home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/dir1/file1.log")
+
.setBaseName("home/claude/apache/creadur-rat/apache-rat-core/target/test-classes/GitIgnoreBuilderTest/src/").build();
+ System.out.println("Decomposition for " + candidate);
+
+ assertThat(matcher.toString()).isEqualTo("matcherSet(or('included
.gitignore', 'included .gitignore'), or('excluded .gitignore', **/.gitignore,
'excluded .gitignore', 'excluded " + globalGitIgnore.substring(1) + "'))");
Review Comment:
Create a document name from the gitignore file name:
```
DocumentName globalIgnoreName = DocumentName.builder(new
File(globalGitIgnoreUrl.getFile())).build();
```
Then use `globalIgnoreName.localized("/").substring(1)`
When dealing with Windows/Linux file name differences `DocumentName` is your
friend.
##########
apache-rat-core/src/main/java/org/apache/rat/config/exclusion/fileProcessors/GitIgnoreBuilder.java:
##########
@@ -102,4 +112,19 @@ protected Optional<String> modifyEntry(final
Consumer<MatcherSet> matcherSetCons
}
return Optional.of(prefix ? NEGATION_PREFIX + pattern : pattern);
}
+
+ /** The location of the global gitignore file to process */
+ protected String globalGitIgnore() {
Review Comment:
This method should return an Optional<File>. Check if any of the files
exist and return the file otherwise return `Optional.empty()` Your
`initializationBuilder` implementation can then handle the case where the file
exists more easily,.
Also, this will give us a single location to enable/disable the use of the
global ignore.
Add a check for "RAT_NO_GIT_GLOBAL_IGNORE" environment var. If it is set
return `Optional.empty()`.
This will allow users to disable this feature if desired.
You will also need to update the documentation for GIT in the
StandardCollection enum. It should mention that the global ignore is active
and can be overridden by setting the environment variable
"RAT_NO_GIT_GLOBAL_IGNORE"
Finally, if you detect RAT_NO_GIT_GLOBAL_IGNORE log an info entry indicating
that global ignore is skipped because of RAT_NO_GIT_GLOBAL_IGNORE
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@creadur.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
