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

Allow passing multiple regex (--not-match) #67

Closed
andyfitzgerald opened this issue Apr 16, 2019 · 16 comments
Closed

Allow passing multiple regex (--not-match) #67

andyfitzgerald opened this issue Apr 16, 2019 · 16 comments
Labels
enhancement New feature or request

Comments

@andyfitzgerald
Copy link

Consider accepting multiple regular expressions instead of just one. Example

scc --not-match ".*\.csv" --not-match "specialDirectory/.*\.txt" .

Currently scc just uses the last specified regex (--not-match) option.

@boyter
Copy link
Owner

boyter commented Apr 17, 2019

Sounds interesting. Would it not be acceptable in this case to have them combined into a single regex though?

scc --not-match ".*\.csv|specialDirectory/.*\.txt" .

The above is not tested BTW

@andyfitzgerald
Copy link
Author

That's certainly an option; however, I propose this as an enhancement to improve readability and maintainability of the code invoking scc.

Imagine a project that embeds scc in a script (make, bash, etc). There will likely be a few variables, or an array variable, containing special cases to ignore. Please see a contrived example below.

SCC_REGEX_EXCLUDES := \
    ".*\.csv" \
    "specialDirectory/.*\.txt" \
    ".*\build.*" \
    "otherSpecialDirectoryPattern" \
    ".*\.hex" \
    "more and more tedious regex.*"
SCC_EXCLUDES_REGEX_STR := $(addprefix --not-match=, $(SCC_REGEX_EXCLUDES))

...

target:
    scc $(SCC_EXCLUDES_REGEX_STR) directoryToScan

In this way, it's easier to read and maintain the script. It also simple to add cases for developers who are not regex experts.

You could argue that the encapsulating script could assemble a regex in a way that's better for maintainability, but I would counter that it's prone to error.

@boyter
Copy link
Owner

boyter commented Apr 17, 2019

That's fair enough. I tend to do multiples when using grep in some situations as well. I am not that familiar with cobra which does the argument parsing but I suspect this should be possible.

@boyter boyter added the enhancement New feature or request label Apr 17, 2019
@andyfitzgerald
Copy link
Author

Assuming that you mean the command line arguments to scc, my guess would be that you could re-use the solution for #61 or what is currently done for --exclude-dir arguments. Multiple --exclude-dir arguments appears to work in my local tests.

@boyter
Copy link
Owner

boyter commented Apr 17, 2019

Neat. I'm having a look at this now.

@boyter
Copy link
Owner

boyter commented Apr 29, 2019

So this works very differently to the way #61 does because that worked outside of Cobra. Not sure if https://github.com/spf13/cobra supports multiple of the same arguments to it. I didn't see anything on the main page anyway. Will have to look into that further.

@dbaggerman
Copy link
Collaborator

Cobra supports StringSlice flags: https://godoc.org/github.com/spf13/pflag#StringSlice

@boyter
Copy link
Owner

boyter commented Apr 29, 2019

Oh nice. Well that answers that question then. I literally just stepped off a plane after 20 hours of flying and didn't feel like digging into the docs.

Ill have a look at implementing this then, unless you want to do it @dbaggerman ?

@dbaggerman
Copy link
Collaborator

I'm busy with other stuff right now, but I can pick it up later if you don't get to it.

@boyter
Copy link
Owner

boyter commented Apr 29, 2019

No worries. My plan is to look into the git ignore issue some more as the main thing for me to tinker with for the next chunk of time I block out for this.

@boyter
Copy link
Owner

boyter commented Apr 30, 2019

@andyfitzgerald This has been merged into master. If you want to try it out you can build from there and give it a go, and all credit to @dbaggerman for actually implementing.

@andyfitzgerald
Copy link
Author

andyfitzgerald commented Apr 30, 2019

I tested this morning with scc version 2.4.0 and found the results identical between ...

  1. single --not-match argument: complex regex string where the different regex are joined with |
  2. multiple --not-match arguments: the same regex strings used above, except passed in as multiple arguments without the | operator

Looks resolved to me. Thanks for the fast work @dbaggerman and @boyter! I'm sure everyone will appreciate the improved readability in their scc wrapper scripts.

@boyter
Copy link
Owner

boyter commented Apr 30, 2019

@andyfitzgerald if you could let me know what test you used ill add it into the test suite. If not no worries ill just make one of my own, but happy to have yours in there.

@andyfitzgerald
Copy link
Author

Thanks for asking. My test cases were ...

single --not-match argument: complex regex string where the different regex are joined with |

scc --not-match="(.*\.hex|.*\.d|.*\.o|.*\.csv|^(./)?[0-9]{8}_.*)" .

multiple --not-match arguments: the same regex strings used above, except passed in as multiple arguments without the | operator

scc --not-match=".*\.hex" --not-match=".*\.d" --not-match=".*\.o" --not-match=".*\.csv" --not-match="^(./)?[0-9]{8}_.*"  . 

Then I simply verified that I got the same file type counts and COCOMO outputs for a directory containing both source files and unwanted files matching those patterns.

@boyter
Copy link
Owner

boyter commented May 1, 2019

Sounds good. Ill add that in now. Thanks for sharing it.

boyter added a commit that referenced this issue May 1, 2019
@boyter
Copy link
Owner

boyter commented May 1, 2019

Added in bfef56a and going to close this down now since all appears well. Thanks for responding with the test case you used.

@boyter boyter closed this as completed May 1, 2019
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

3 participants