FIX: Respect exclude option in config file #111
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
danielhollas
changed the title
I also added a bunch of tests to test this functionality. The new tests also cover the fix for fortran-lang#94.
danielhollas
force-pushed
the
fix-exclude-config
branch
from
40541a4
to
d9219a6
Compare
danielhollas
commented
May 27, 2021
| @@ -2046,6 +2056,19 @@ def build_ws_dict(args): | |||
| ws_dict['intrinsics'] = args.whitespace_intrinsics | |||
| return ws_dict | |||
|
|
|||
| def build_case_dict(args): | |||
There was a problem hiding this comment.
Just a bit of unrelated refactor to make this consistent with build_ws_dict().
danielhollas
commented
May 27, 2021
| " does not exist!\n") | ||
| sys.exit(1) | ||
| if not os.path.isfile(directory) and directory != '-' and not args.recursive: | ||
| sys.stderr.write("file " + directory + " does not exist!\n") | ||
| if os.path.isdir(directory) and not args.recursive: |
There was a problem hiding this comment.
I think this is a bit clearer. Also the directory != '-' condition was redundant here.
danielhollas
commented
May 27, 2021
| # Prune excluded patterns from list of child directories | ||
| # https://stackoverflow.com/a/19859907 |
There was a problem hiding this comment.
I was fairly confused by this code so added a link.
danielhollas
commented
May 27, 2021
| @@ -2077,35 +2101,34 @@ def build_ws_dict(args): | |||
|
|
|||
| for dirpath, dirnames, files in os.walk(directory,topdown=True): | |||
|
|
|||
| file_args = args | |||
danielhollas
commented
May 27, 2021
| else: | ||
| self.removeTmpDir() | ||
|
|
||
| def test_config_stdin(self): |
There was a problem hiding this comment.
This is a test for the fix in #98.
danielhollas
commented
May 27, 2021
| p1.wait() | ||
|
|
||
| check_output_file(alien_file, outstring_with_config) | ||
| # Excluded files and directories should not be touched at all. |
There was a problem hiding this comment.
This part of the test would fail before the fix.
|
@pseewald friendly ping. 😊 LMK if you want me to split this PR into smaller pieces to make it easier to review. |
|
I've approved the run but I won't have time until the end of the year to become familiar with this PR and review changes. |
danielhollas
added a commit
to danielhollas/fprettify
that referenced
this pull request
Oct 29, 2023
Upstream PR: github.com/fortran-lang/pull/111
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As explained in #110, the
excludeoption given via the config file did not work.Here's the fix, a bit of a refactor, and a bunch of new tests.
If this change looks good to you, I would do a bit more refactoring since right now we're needlessly re-parsing the config files for each FORTRAN file, instead of just parsing it once per directory. I didn't want to do it right away to make the initial review easier.
I build this PR on top of #98 since it touched the same code. I am happy to rebase if it gets merged first.
Closes #110.
CC @pseewald
sidenote: Instead of the
excludeoption, it would be nice to support something like the.gitignorefile. For example, ESLint has.eslintignore.