feat: option to add additionalPlainTextMasks

[dpozinen]

What's changed?

Add rewrite.additionalPlainTextMasks plugin option

What's your motivation?

The default list is super long and it was impossible to extend it - only override, which meant users would need to copy paste it + add their additional masks

[timtebeek]

Thanks a lot for this improvement @dpozinen ! That override had been a pain to use for a long time.

Weighing the options now I wonder when folks would want to continue to use the old plainTextMasks if additionalPlainTextMasks is available. 🤔 I don't really see a good use case, beyond perhaps a flimsy "I want a known masked file as Quark instead of PlainText". Do you have any thoughts on that?

If we can't come up with a good use case for the "old" masks field, then we could perhaps look at completely replacing or repurposing that one, as opposed to adding an additional flag.

[dpozinen]

Thanks a lot for this improvement @dpozinen ! That override had been a pain to use for a long time.

Weighing the options now I wonder when folks would want to continue to use the old plainTextMasks if additionalPlainTextMasks is available. 🤔 I don't really see a good use case, beyond perhaps a flimsy "I want a known masked file as Quark instead of PlainText". Do you have any thoughts on that?

If we can't come up with a good use case for the "old" masks field, then we could perhaps look at completely replacing or repurposing that one, as opposed to adding an additional flag.

Hi, yeah, I though about that too and figured maybe people would like to limit the scope of their runs for big monorepos, for example excluding .py files. Other than that, you're right, there isn't really a usecase. I'm fine with replacing, but it doesn't seem like a pain to support, and maybe someone is already using that, so it would be a breaking change

[timtebeek]

Hi, yeah, I though about that too and figured maybe people would like to limit the scope of their runs for big monorepos, for example excluding .py files. Other than that, you're right, there isn't really a usecase. I'm fine with replacing, but it doesn't seem like a pain to support, and maybe someone is already using that, so it would be a breaking change

Yes since it's sets we're using here I figured we could fairly silently switch over from interpreting plainTextMasks as replacement to plainTextMasks as additional masks, assuming folks typically include the original plaintext masks and add their own.

That said, there have been cases in the past where folks blocked out excessively large XML files in their repositories using a plain text mask override, so perhaps we shouldn't spring that on folks. 🤔

[dpozinen]

Yes since it's sets we're using here I figured we could fairly silently switch over from interpreting plainTextMasks as replacement to plainTextMasks as additional masks, assuming folks typically include the original plaintext masks and add their own.

That said, there have been cases in the past where folks blocked out excessively large XML files in their repositories using a plain text mask override, so perhaps we shouldn't spring that on folks. 🤔

Yeah, that's what I'm saying is this would remove the ability to reduce the scope, which might be valuable for some

[timtebeek]

Thanks for the addition and helpful discussion weighing the options!