Apply .gitignore correctly in every source entry #3336
Conversation
|
I'm not sure about why the checks failed 🤔 These seems to be not related with my changes, maybe this requires running the checks again?
EDIT: NVM, got the actual problem: |
aaossa
force-pushed
the
issue-3291
branch
4 times, most recently
from
556fa41
to
8475676
Compare
When passing multiple src directories, the root gitignore was only applied to the first processed source. The reason is that, in the first source, exclude is `None`, but then the value gets overridden by `re_compile_maybe_verbose(DEFAULT_EXCLUDES)`, so in the next iteration where the source is a directory, the condition is not met and sets the value of `gitignore` to `None`. To fix this problem, we store a boolean indicating if `exclude` is `None` and set the value of `exclude` to its default value if that's the case. This makes sure that the flow enters the correct condition on following iterations and also keeps the original value if the condition is not met. Also, the value of `gitignore` is initialized as `None` and overriden if necessary. The value of `root_gitignore` is always calculated to avoid using additional variables (at the small cost of additional computations). Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>
The test creates a fake context and collects the files from two sources. Both sources are directories in the root path, where a gitignore file ignores a filename that is present in both subdirs. Before the fix introduced in the previous commit, this test was expected to fail: a file that should be ignores was still visible for Black. Now, the test is passed. Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>
Add entry about fixed bug: .gitignore being skipped when more than one source directory was given Signed-off-by: Antonio Ossa Guerra <aaossa@uc.cl>
There was a problem hiding this comment.
Hey I just wanted to say thanks for the PR. I really appreciate it! Also thank you @JelleZijlstra for the review! 🧡
The PR overall looked good, but two things stood out to me on a quick scan.
I'll tackle your suggestions while solving conflicts on #3338 . One of the conflicts is around the same area, so solving them there is "free" |
Description
Fix #3291 : incorrectly ignoring .gitignore presence when more than one source directory is specified. Commit message of the commit that fixed the bug:
When passing multiple src directories, the root gitignore was only applied to the first processed source. The reason is that, in the first source, exclude is
None, but then the value gets overridden byre_compile_maybe_verbose(DEFAULT_EXCLUDES), so in the next iteration where the source is a directory, the condition is not met and sets the value ofgitignoretoNone.To fix this problem, we set the new value of
excludeis a new variable (_exclude). This prevents entering the wrong condition on the second iteration and also keeps the origina value if the condition is not met.Also, the value of both
_excludeandgitignoreis calculated only once by usingNoneas default value. This help us to reduce the number of computations (only calculate the value once, if needed), and also retains the original value ofgitignore, which is then merged with the value ofp_gitignore. That's why we also define_gitignoreto contain the result of the merge (add) operation.Checklist - did you ...
CHANGES.mdif necessary?Add new / update outdated documentation?