search

mganss / HtmlSanitizer

Cleans HTML to avoid XSS attacks
MIT License
1.52k stars 198 forks source link

Rename namespace #376

Closed vanillajonathan closed 1 year ago

vanillajonathan commented 1 year ago

Rename namespace from Ganss.XSS to Ganss.Xss as per the Microsoft .NET Framework Design Guidelines Capitalization Conventions.

vanillajonathan commented 1 year ago

@mganss You okay with this?

tiesont commented 1 year ago

@vanillajonathan Just my opinion, obviously, but I don't know that this is really required. .NET has namespaces that violate the stated convention already, like System.IO. XSS is an acronym, not a proper word, so I'd expect it to be written as-is.

Of course, there are namespaces like System.Web.Security.AntiXss, so perhaps the precedent has been established to use the casing you're proposing. 🤷‍♂️

mganss commented 1 year ago

I'm hesitant on this. Not only would this be a breaking change, but it would require recompilation for all users. If I were faced with this change as a user, I'd probably think 🙄

vanillajonathan commented 1 year ago

@tiesont No System.IO does not violate the .NET naming guidelines because IO is a two-letter acronym and the the naming convention does not apply to two-letter acronyms, only three letter and longer. The linked page explains this.

@mganss Yes, this would a breaking change, but so was PR #370. They're both breaking changes. Introducing these breaking changes should be since they're introduced together at the same time for version 8.

mganss commented 1 year ago

I know this would be a good opportunity to introduce breaking changes. The issue I'm having with this change is that it's not about features but "just" a naming change, in particular one that might confuse users. When they update the NuGet package and recompile, they'll get errors which are not obvious to fix.

vanillajonathan commented 1 year ago

Modern IDEs autodiscover referenced classes and provides suggestions for using statements. You hover over the HtmlSanatizerin the code and it presents you with a yellow light bulb that says "Import Ganss.Xss", this should be trivial for users to fix.

tiesont commented 1 year ago

If this is the one and only thing in the project that violates the naming convention, I suppose it's at least worth considering. Does feel like like a rather pedantic change, if I'm being honest. I say that with absolutely no malice or ill will intended.

vanillajonathan commented 1 year ago

I am not aware of anything else that violates the naming convention. I agree that it is a pedantic change, but it would nice to "get it right" when there is an API breakage anyway in a new major version 8 release.

mganss commented 1 year ago

🆗 Let's do it then.