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

scc includes files that I've excluded via --not-match #161

Closed
beaugunderson opened this issue Mar 6, 2020 · 5 comments
Closed

scc includes files that I've excluded via --not-match #161

beaugunderson opened this issue Mar 6, 2020 · 5 comments
Labels
enhancement New feature or request

Comments

@beaugunderson
Copy link

Describe the bug

scc includes files that I've excluded via --not-match.

To Reproduce

mkdir testing
pushd testing

mkdir schemas
mkdir migrations

echo "TEST\nTEST\nTEST\n" >> schemas/_nsgroup.py
echo "TEST\nTEST\nTEST\n" >> migrations/migration.py

scc \
  --by-file \
  --no-gen \
  --include-ext=py,js,ts,tsx \
  --not-match 'schemas/.*\.py' \
  --not-match 'migrations' \
  --not-match 'message_pb' \
  --not-match 'registerServiceWorker' \
  --not-match 'introspect-schema' \
  --exclude-dir 'types' \
  --exclude-dir 'vendor'

The output includes schemas/_nsgroup.py, despite being excluded via --not-match.

───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
Python                       1        34        0         0       34          0
───────────────────────────────────────────────────────────────────────────────
schemas/_nsgroup.py                   34        0         0       34          0
───────────────────────────────────────────────────────────────────────────────
Total                        1        34        0         0       34          0
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop $2,204
Estimated Schedule Effort 1.009153 months
Estimated People Required 0.091043
───────────────────────────────────────────────────────────────────────────────

Expected behavior

schemas/_nsgroup.py should not be included.

Desktop (please complete the following information):

  • OS: macOS
  • Version: 10.15.3
  • SCC version: 2.12.0
@beaugunderson
Copy link
Author

beaugunderson commented Mar 6, 2020

also doesn't handle this case:

  --not-match 'cqms/v20\d\d\.py' \

for files named value_sets/cqms/v2019.py, value_sets/cqms/v2020.py, etc.

even

  --not-match 'value_sets/cqms/v2019.py' \

does not work...

boyter added a commit that referenced this issue Mar 8, 2020
@boyter
Copy link
Owner

boyter commented Mar 8, 2020

Oh that's annoying.

So the issue is that the exclusion is only applying to the name of the directory/file and NOT the full path, which is what you are expecting.

I can understand the confusion because the help text says,

-M, --not-match stringArray       ignore files and directories matching regular expression

which implies it would be the full path, however it is not. I think the thinking at the time was that generally this sort of case would be solved with either a .gitignore or .ignore file. That said it seems like it should be fixed. A patch which resolves the issue,

diff --git processor/file.go processor/file.go
index d8875d3..3e4c60a 100644
--- processor/file.go
+++ processor/file.go
@@ -145,9 +145,9 @@ DIRENTS:
                }

                for _, exclude := range dw.excludes {
-                       if exclude.Match([]byte(name)) {
+                       if exclude.Match([]byte(name)) || exclude.Match([]byte(path)) {
                                if Verbose {
-                                       printWarn("skipping directory due to match exclude: " + name)
+                                       printWarn("skipping file/directory due to match exclude: " + name)
                                }
                                continue DIRENTS
                        }

then gives the following output for your test case,

$ scc \
>   --by-file \
>   --no-gen \
>   --include-ext=py,js,ts,tsx \
>   --not-match 'schemas/.*\.py' \
>   --not-match 'migrations' \
>   --not-match 'message_pb' \
>   --not-match 'registerServiceWorker' \
>   --not-match 'introspect-schema' \
>   --exclude-dir 'types' \
>   --exclude-dir 'vendor'
───────────────────────────────────────────────────────────────────────────────
Language                 Files     Lines   Blanks  Comments     Code Complexity
───────────────────────────────────────────────────────────────────────────────
Total                        0         0        0         0        0          0
───────────────────────────────────────────────────────────────────────────────
Estimated Cost to Develop $0
Estimated Schedule Effort 0.000000 months
Estimated People Required 1 Grandparent
───────────────────────────────────────────────────────────────────────────────

I am going to commit the change now e8ca7ea so if you build from master you will get it. I am then going to beef up the tests around this and ensure it gets rolled into the next release.

@beaugunderson
Copy link
Author

ah, cool! i did not realize .ignore was a thing, and you're right that for these cases it would also work. would suggest documenting that more since i did look in the source/readme a bit and didn't find it. appreciate the quick fix for --not-match as well. :)

@boyter
Copy link
Owner

boyter commented Mar 9, 2020

Yes the .ignore file is something I think I assume most people know about... but only those familiar with other tools might be. Ill add some documentation around it.

@boyter boyter added the enhancement New feature or request label Aug 3, 2020
@boyter
Copy link
Owner

boyter commented Aug 3, 2020

Documentation added here 09448a6

@boyter boyter closed this as completed Aug 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants