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

Regex-Support #82

Draft
wants to merge 5 commits into
base: master
Choose a base branch
from
Draft

Regex-Support #82

wants to merge 5 commits into from

Conversation

kimpenhaus
Copy link

Hi @realvizu

I tried to integrate Regex support as requested in #53 - this is by far not perfect and needs at least some documentation on the RegexDomain class itself. I tried to adhere some standards as I forced the regex to have a / delimiter at start and end (similiar to js, perletc)

But the two main problems I have - and which I am asking for some help - are the unit-tests and the nuget/dotnet pack execution.

I am running on an osx machine which seems to struggle with some of the commands:

grafik

might be some issues with slashes/backslashes in specific os environments...

Maybe you can give me some support in points of issues - and give me some feedback on what I should change in the codebase.

Thanks.

@realvizu
Copy link
Owner

realvizu commented Feb 2, 2025

Hi!

First of all, thanks for contributing to NsDepCop!

Regarding the unit tests failing on OSX: that's my bad, I did not pay attention to make the path handling in the tests cross-platform. I've added a commit to your branch, please check out if it solves the problem.

Regarding the nuget/dotnet pack execution: please provide some details what kind of error do you get.

Regarding the feature implementation: I think it's a good idea to use the slash begin/end delimiters for regex rules, so they are easily distingushed from the normal and the wildcard rules. My main concern with regex rules is the possible performance impact, and this implementation ensures that the normal and wildcard rules are not affected in any way, so it's great.

Regarding the GetMatchRelevance method implementation: its purpose is to find the "best" matching rule in case there are multiple matching ones. Currently the Domain class (that encapsulates a concrete namespace) is considered highest precedence, followed by the WildcardDomain, which returns a lower GetMatchRelevance value, based on how many wildcard substitutions were needed to make a match. In order to be backward compatible, the RegexDomain should be the last in the rules precedence order, so it should return a GetMatchRelevance value which is always smaller then the ones given by Domain and WildcardDomain. I'm not sure how well I explained it so feel free to ask for more clarification is needed. Also, please add some unit tests that check that in case both a WildcardDomain and a RegexDomain rule gives a match, then the WildcardDomain is chosen as best matching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants