Add HtmlSanitizerOptions

[vanillajonathan]

No description provided.

[codecov]

Codecov Report

Merging #359 (b21e34a) into master (874f1be) will increase coverage by 0.11%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #359      +/-   ##
==========================================
+ Coverage   94.32%   94.44%   +0.11%     
==========================================
  Files           4        5       +1     
  Lines         811      828      +17     
  Branches       85       85              
==========================================
+ Hits          765      782      +17     
  Misses         34       34              
  Partials       12       12              

Impacted Files	Coverage Δ
src/HtmlSanitizer/HtmlSanitizer.cs	96.17% <100.00%> (+0.05%)	⬆️
src/HtmlSanitizer/HtmlSanitizerOptions.cs	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 874f1be...b21e34a. Read the comment docs.

[mganss]

Shouldn't these have private setters? Also, they should be initialized with StringComparer.OrdinalIgnoreCase.

[vanillajonathan]

No, I would want public setters because then I can easily assign a set to the properties using object initializer syntax. Without a setter, I would have to loop over a set and append each item in the set using the Add method.

Yes, it should probably be initialized with StringComparer.OrdinalIgnoreCase if we want it to be case-insensitive, which we probably do want.

[mganss]

You can use object and collection initializer syntax even if the setter is private. Collection initializer syntax works with the Add() method so you can do this:

public class SetTest
{
    public HashSet<string> Set { get; private set; } = new HashSet<string>();
}

var s = new SetTest()
{
    Set = { "a", "b" }
};

[vanillajonathan]

Yes. but you cannot assign from a predefined set like this:

public static class Presets
{
    public static HashSet<string> Default { get; } = new HashSet<string>();
}

var s = new SetTest()
{
    Set = Presets.Default
};

[mganss]

OK, then how about making them init-only?

[vanillajonathan]

Good idea! I like that!

[mganss]

There's this problem now. Not sure how to deal with it or if it's even worth it.

[vanillajonathan]

It seems easier to just skip init and just have public setters instead.

[mganss]

Yeah ok, I agree.

[mganss]

Thanks for starting this. I'm wondering how the HtmlSanitizerOptions should tie in with the static DefaultAllowedTags etc. properties that are already in HtmlSanitizer. Should there be an additional DefaultOptions property or should it replace the individual properties? cc @tiesont

[tiesont]

Thanks for starting this. I'm wondering how the HtmlSanitizerOptions should tie in with the static DefaultAllowedTags etc. properties that are already in HtmlSanitizer. Should there be an additional DefaultOptions property or should it replace the individual properties? cc @tiesont

I think you'd wind up with a simpler process for setting up a sanitizer if this just completely replaces the existing properties.

[vanillajonathan]

I agree with @tiesont.

[vanillajonathan]

Sorry, I had forgot about this. It builds now.

[mganss]

Could you adapt the unit tests to cover the changes?

[vanillajonathan]

Now it says there is coverage.