Squelch reports

[balthisar]

Adds message squelching (muting) to Tidy, avoiding the need to specify a new option every time we might want to allow or disallow new output. Adds two new options:

• squelch-id (default no) will provide tags during Tidy's output that
  can be used to silence warnings. For example, MISSING_ENDTAG_OPTIONAL
  will be shown after warnings of that type. Then in the future, the user can
  use...
• squelch (default NULL) takes a list of message identification tags.
  Messages of this tag type will then be muted from output.

This gives the user great control over hiding things that he or she doesn't care
about seeing. However, muted errors/warnings still count toward error summaries.

If we merge this, then TidyShowMetaChange and TidyWarnPropAttrs are candidates
for removal (using the deprecation mechanism Tidy now has, these will continue
to work, though).

[balthisar]

I'll fork the test repo to add a #629 test case, too, which will provide an example.

[geoffmcl]

@balthisar wow, this sort of seems like an out-of-the-blue Feature Request ;=))

I have now had a chance to checkout this squelch_reports branch, and test it using the pr_629 branch of tests... and it works... thanks...

You say the reason for this is - This gives the user great control over hiding things that he or she doesn't care about seeing. - sounds good... no problem...

Is this a Feature Request by you, or do others feel the need for this as well? Please ponit me to the issues...

I am afraid I still do not see any use case where specific messages need to be suppressed, other than the control we already have...

Maybe you can open a Feature Request issue to discuss this further...

And the name, squelsh! Yes, in the lexicon of this word, there is a sense of suppress, maybe mute, but in general, at least in English dictionaries, as opposed to American, is has -

1. sound treading through mud
2. electronic: suppresses signals below certain level

And wikipedia seems to concentrate on the second meaning...

Yes, in say Merriam-Websters, I can find the senses of quell or silence... usually resistance or protests...

Then it seems you want the user to know the message identification name, internal to the library, like MISSING_ENDTAG_OPTIONAL. Where are these published, except in the source, in language_en.h, and tidyenum.h?

Assume you intend to massively expand the public API, so that I could find a message to be suppressed, and thus find its identification name, to even be able to use this squelch: <name> option... users should not have to search source code...

Finally, tidy library user can already install a message filter, which you also added, to filter out what they do not want...

You can see, at this moment, without lots more discussion, and reasoning, hopefully in a feature request issue, I do not see the reason for this... sorry...

Ok, back here later - I always prepare my replies offline - and trying to be positive...

First still against the choice of the option name, squelsh!

If I was searching for options to suppress, mute, silence, quell, stop, inhibit a particular message, I do not think I would ever think of squelsh! Maybe something in my Australian English education, but I have travelled, and worked, all over the world... But ok assume a better choice of the option name is chosen...

Really what is the use case, given that I understand it will still count in the warning/error count?

We can use -q to quiet them all... and there is a limiter, --show-error NN, and given that a library user can filter messages - The filter has had a chance to suppress *any* message from output.... what more is needed?

Is this just a way to retire at some point, TidyShowMetaChange and TidyWarnPropAttrs, and stop more of these? But then you add more code to replace these...

I guess I should stop here. It goes without saying, I do not support this PR, at this time...

Maybe more feedback will make it clearer... maybe I am just confused... thanks...

[balthisar]

"squelch," not "squelsh," and as an electrical engineer, yeah, it means to silence (specifically it's to allow a certain signal threshold). Hmmm... maybe doesn't sound as good to non-American ears, but trivial to change to "mute" or "silence" or whatever. In our lingo, one might say "squelch that idea," but also in our lingo to "be pissed" is "be upset," so it's no surprise that it doesn't resonate in other regions.

Console users don't use the library, so they have no ability to filter. This offers a nice compromise between "I don't want to see that warning!" and "Show me everything!"

Yes, the idea is to eliminate TidyShowMetaChange and TidyWarnPropAttrs, but more importantly, to prevent new options like them from ever having to appear again. These two new options avoid all of that nonsense in the future. Even if we don't retire those two options (a different conversation!), this gives users flexibility without breaking anything. Breaking things is a different conversation.

Oh, two new options. Try running the PR with the quelch-id option. No studying the source code is necessary. :-)

And, not really a new feature request, but kind of the backbone to #607 and #609.

[balthisar]

Nomenclature

I've managed to push updates naming the options mute (for muting) and mute-id (for discovery).

Use Cases

Maintenance

This PR avoids a lot of future maintenance by avoiding the need to add new configuration options every time we want to add a new warning, but are afraid people won't like new warnings.

Precedent

For example TidyShowMetaChange was added, for what purpose? It doesn't protect against regressions for what Tidy produces; it only protects for regressions on the error output. Why do we care about error output, as long as it's not erroneous? If there's a case for hiding this particular message, then there's a case for the user to want to hide any message. Similar for TidyWarnPropAttrs; why do we feel that it's valid to accept that option but not offer silencing any option?

Other Feature Requests

#477, potentially generates changed output user might not want to see.
#505, potentially generates changed output user might not want to see.
Any other future feature requests that affect output...

Infrastructure

Infrastructure makes things more manageable for future maintenance, and eliminating options (either now or in the future) makes things more maintainable in the long run. This is primarily an infrastructure addition, while giving users a nice new feature.

User acceptance

Gives console application users something new, out of the box, without having to build their own libtidy applications. Tidy needs to be flexible for users, and we want people to want to use Tidy rather than have to use Tidy. That's means we sometimes do more than fix existing bugs.

Impact

It does not remove any additional options, or change any output (HTML or messages) any way on its own, unless activated knowingly.

[geoffmcl]

@balthisar ok, warming to the idea ;=)) with the name change to mute, and I had completely miss-read the meaning of the --mute-id yes option... now exploring with my tidy 5.5.63.I629...

Two things I found -

1. It seems it only works on warnings and errors?
2. The noisy Config: ... output seems not suppressed by -q?

The output of Info: Document content looks like HTML5 (STRING_CONTENT_LOOKS), and I add --mute STRING_CONTENT_LOOKS, will give me Config: option "mute" given bad argument "STRING_CONTENT_LOOKS". So is muting of Info: ... messages not possible, except by --show-info no?

And although I added -q I still see -

Config: messages of type "MISSING_DOCTYPE" will not be output
Config: messages of type "INSERTING_TAG" will not be output
Config: messages of type "MISSING_TITLE_ELEMENT" will not be output

I guess I expect -quiet to really be quiet?

Still exploring... thanks for the additional comments...

[balthisar]

@geoffmcl, good catch with STRING_CONTENT_LOOKS; it falls outside of the range of REPORT_MESSAGE_FIRST and REPORT_MESSAGE_LAST, so it's being rejected, even though we've assigned TidyInfo to that. I've just pushed a quick fix.

I'll look at the -q option. I think this option -- even without this PR -- doesn't silence config category messages, but until now they've been uncommon and you've simply not noticed.

Recently I've made some meticulous, in-depth study into all of the conditions that cause all message output, and made several internal code changes to manage these conditions centrally, rather than spreading conditions all over code. I'm pretty sure that all LibTidy output decisions (not console output) are managed completely in messageOut() now. All that remains is actually quite simple!

• Accept some slight changes to regression error output.
• Decide what categories of messages we want -q to silence.
• Ensure that messages are in the correct category.

Basically I think -q should turn off everything except for TidyWarning/TidyError/TidyInfo level messages. This is a trivial fix, but I didn't roll it into this PR because it does break some existing cases on errout.

[geoffmcl]

@balthisar, thanks, STRING_CONTENT_LOOKS has now been fixed...

I guess we have a different idea about changing the errout messages!

As you correctly point out, I added TidyShowMetaChange specifically, and only to maintain the existing errout regression test messages... The idea being that there would be a period where people could get used to these new messages... Then maybe we later switch the option default to yes, and again wait for maybe a few release cycles... And after all that patience, maybe remove the option... A steady, careful, measured progression...

But a recent PR, this?, or another?, you at the same time changed some 30 plus expected messages... This completely tosses out my very cautious approach...

I am trying hard to get over my caution ;=)) Maybe you are right! errout messages should not be considered important... but I still sit on the fence...

And certainly agree -q, if given early in the command, should also suppress Config:... messages. Did not test, but could only find one testbase-expects/*.txt that has a Config: message, case-431716, but does not have -q, so should not change... but as I say, not fully tested...

But ok, let's go for it ;=)) We may or may not get blow-back when we release 5.6, hopefully next month...

[balthisar]

I'll merge this base on comments above, and will update the -q option separately. Right now, can't test the #604 fix, because it's already based on this branch!