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

Message Granularity #607

Open
balthisar opened this issue Sep 19, 2017 · 2 comments
Open

Message Granularity #607

balthisar opened this issue Sep 19, 2017 · 2 comments
Assignees
Milestone

Comments

@balthisar
Copy link
Member

Now that every message has been inventoried and binned into an appropriate category (not a lot of changes, but a necessary audit), and the message system is mostly very, very modular and flexible now, I'd like to propose some changes to enhance the granularity of messages that Tidy outputs. This will break existing diffs and introduce/deprecate some existing config options, but a little bit of pain brings a lot of consistency to Tidy's output.

From the old, closed #561:

The TidyInfo isn't used very much; perhaps it should be used a little bit more for cases like this, i.e., "silent changes" become TidyInfo messages that we can handle appropriately.

There are often issues raised about whether or not Tidy should emit or not emit a message based on silent changes, things we don't want to see, etc. Although @geoffmcl indicates that he added TidyInfo as a quick fix, I say, let's embrace this. If something isn't an error or warning, then it's appropriate to potentially output it as TidyInfo. For example, the new TidyShowMetaChange added an entire new option for whether or not to show this change as a TidyInfo message. While this is granular control, and this issue is about adding granular control, I think it would be sufficient to delete this option and simply count on hiding TidyInfo if the user doesn't want to see this level of message.

Also from #561:

  • show-info will squelch any TidyInfo messages, but it also squelches some of the "dialogue-like" output that Tidy delivers (the new TidyDialogueInfo) category.
  • show-warnings can be used to hide the warnings
  • show-errors isn't a boolean, but places a limit to the amount of errors that will be output, and setting this to zero does squelch all errors. Now the thing about show-errors is that it only limits errors, not warnings or info. So if you set show-errors to, say, 10, but have 100 warnings, you'll still see all of them.
  • If you're using a callback filter, none of these options have any effect, as it becomes the responsibility of the host program to implement all of this functionality.

Here's the current state of options that affect output:

OptID Option Effect
TidyShowErrors show-errors Limits the number of errors (only) to show.
TidyShowInfo show-info Hides TidyInfo messages (only)
TidyShowWarnings show-warnings Hides TidyWarning messages (only)
TidyQuiet quiet This option specifies if Tidy should output the summary of the numbers of errors and warnings, or the welcome or informational messages. It's also suppressing footnotes. It's mostly used to squelch console output, and in tidyRunDiagnostics to squelch TY_(ReportMarkupVersion)( doc ) and TY_(ReportNumWarnings)( doc ). We should classify these messages as a different report level, and worry about squelching them. Used in triggering tidyErrorSummary (the footnotes) and tidyGeneralInfo.
TidyShowMetaChange show-meta-change WTF? Why a whole option for this? If used, it's TidyInfo, so why not just count on TidyInfo?

Proposed replacement:

OptID Option Type Effect
TidyLimitReports limit-reports Int Limits the number of any reports, in the order they are found, where 0 is all.
TidyShowReports show-reports Enum None, Errors, Warnings, Info, default is Info, i.e., it shows everything up to and including TidyInfo. Set it to Warnings, and it only shows Errors and Warnings. Etc.
TidyShowFootnotes show-footnotes Enum None, All, Condensed, default is Condensed. Until "real" footnotes are working, enum is None or Condensed. Further explanation below.
TidyQuiet Quiet Bool If quiet, turns off everything except report messages as configured above.

Alternatively, simply keep TidyShowErrors, but give it the new behavior as described for TidyLimitReports.

Footnotes: right now, several content-related helpful messages really are footnotes; they're triggered by the bit fields encountered during parsing or cleaning. This is a really useful feature, and I'd like to expand on this a bit in the future. In the meantime, this option would turn these off or on, and TidyQuiet would override this setting if TidyQuiet is used, i.e., one TidyQuiet squelches everything except reports at the selected report level.

Other TidyReportLevel handling:

Level How Treated per the above
TidyInfo Hidden according to TidyShowReports
TidyWarning Treated as TidyWarning
TidyConfig Treated as TidyError
TidyAccess Treated as TidyWarning
TidyError Treated as TidyError
TidyBadDocument Treated as TidyError
TidyFatal Treated as TidyError
TidyDialogueInfo Hidden with TidyQuiet.
TidyDialogueSummary Hidden with TidyQuiet.
TidyDialogueFootnote Hidden according to TidyShowFootnotes, or TidyQuiet
TidyDialogueDoc = TidyDialogueFootnote Deprected, same as TidyDialogueFootnote

It's a fairly simple change at this point, and I'll see about adding a PR just for preview purposes in case none of this makes any damned sense in text!

tl;dr

This change would give us:

  • Control over total reports generated (not just errors)
  • Control over the report detail with a single option ( info, warning, error )
  • Control over footnote-like text separately from other noise.
  • The ability to control all non-report noise.
@geoffmcl
Copy link
Contributor

@balthisar I have tried to comment on this several times, and always get into a too long mess! This is still too long, but is the shortest so far...

While your tl;dr summary reads good, invoking a yes, that is what we want, my first thought was but is this not what we have now, more or less?. Not perfect, but par contre, not too/so bad...

Reading, and re-reading, the text above the summary does not seem to clarify exactly what you want to do! What will really be changed? How will that improve the situation? A branch/PR may certainly help...

It seems clear you want to replace TidyShowErrors(b)/TidyShowInfo(b)/TidyShowWarnings(b)/TidyShowMetaChange(b) with TidyLimitReports(int)/TidyShowReports(enum)/TidyShowFootnotes(enum), and maybe modify TidyQuiet(b), but not sure about that exactly... Hmmm, wow, I really do not see the full purpose here... sorry...

And then you follow this with #608 - formalize footnotes, and #609 - cleanup config... again, while everybody can agree with the simple sentiment in their repectvie titles, the final results still seem unclear for me...

The end result, which ever way you look at it, is considerable backward compatibility will be lost!

And a small word about TidyShowMetaChange. I introduced this only to maintain compatibility...

And while I can see the new nomenclature of footnotes standing for the bit field of TY_(ErrorSummary), do not see it as a simple replacement of Info messages, which includes tidyGeneralInfo... these seem different... but...

At the very least I would ask that such changes, if it is what is desired, be moved to a milestone beyond the next 5.6 release of Nov 23, 2017... In other words, such message type/groups changes, and config changes be introduced immediately after the 5.6 release, and we have time before the next release to deal with what ever crops up...

In other words, such a comptibility change should come at the beginning of a development cycles, not dropped in virtually weeks before a release...

Alternatively, maybe we could add a new tidy6 console app, shipped with tidy, so users can try it, and give us feedback before that is the only app/lib shipped, re-named to tidy... just an idea...

I do want change, improvement, and I am sure many others do too, but we, as the current maintainers, should not make ourselves the sole arbiter of change, no matter how good, logicial, it is... Given the current 17 year history, we should give a year or more, to get feedback... If little or none, as has been the recent case, then we proceed, else we find a compromise that suits most...

For some reason I feel like I am raining on your parade... sorry about that... maybe I am too old, and conservative... but if I had a current compatible tidy-5.6, and a new tidy6 to play with, I am sure I can adapt...

Meantime, there seems a large number of issues, bugs, feature requests, ... to deal with, but I understand voluntary work leads us to do what we are most interested in...

I certainly hope we get more feedback on this topic... thanks...

@balthisar
Copy link
Member Author

@geoffmcl, I'll whip up a quick PR if I have time this weekend. It actually wouldn't break backwards compatibility the way I'd make it work, though. And, no raining on my parade, as I've been working on some bug fixes. Unfortunately I'm stuck with MSVC 10 for the time being, and the unfamiliarity with it makes things tedious. So, no fun PR's until I have my Xcode back.

@geoffmcl geoffmcl modified the milestones: 5.5, 5.7 Nov 28, 2017
@balthisar balthisar modified the milestones: 5.7, 5.9 Jul 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants