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

Question about ResideInAssembly #245

Open
pawlos opened this issue Mar 23, 2024 · 2 comments · May be fixed by #251
Open

Question about ResideInAssembly #245

pawlos opened this issue Mar 23, 2024 · 2 comments · May be fixed by #251
Labels
kind/feature Categorizes issue or PR as related to a new feature.

Comments

@pawlos
Copy link

pawlos commented Mar 23, 2024

I've been playing with the library and encountered an issue with the above mentioned method.

The signature of it is

ResideInAssembly(string pattern, bool useRegularExpressions = false)

So my assumption was that it should match an assembly by name, with either using a regular expressions or w/o, just by simply comparing parts of the name.

To my surprise that later option didn't work no matter what parts of assembly name I would give.

Today, I dig through the code and discovered that in case of non-regular expression version the match is by the full assembly name. It was a bit counter intuitive for me as the pattern variable name would suggest that it's should match less restrictively (maybe with StartsWith?). The pattern parameter name makes sens for RegEx option but not (for me) for the non-regex one.

At least a /// <remark> would be nice to be there to indicate that in case of non-regex one it should be a full assembly name. If that's ok I could create PR with adding an info or maybe it's ok to somehow redesign this to me more clear what's the intent of the method? Or maybe it should actually match FullName of an assembly using a .StartsWith?

@alexanderlinne
Copy link
Collaborator

Hi @pawlos, if you'd like to create a PR with a documentation comment we'd welcome that.

We are currently also discussing a rework of these APIs to use separate functions like ResideInAssemblyWithName and ResideInAssemblyWithNameMatching instead of a bool parameter. Another option would be ResideInAssembly(Assemblies().That().HaveNameStartingWith("System")). Of course this would be a larger change so this will probably not be implemented soon.

@alexanderlinne alexanderlinne added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 12, 2024
@pawlos
Copy link
Author

pawlos commented Apr 15, 2024

hello @alexanderlinne

Thanks for your reply. Restructuring the API would be the best but I do get that it's the larger change. I've pushed a small PR to add this extra info. Please let me know if I can improve it in any way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants