-
Notifications
You must be signed in to change notification settings - Fork 425
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
Comments
@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 Reading, and re-reading, the text above the It seems clear you want to replace And then you follow this with #608 - formalize The end result, which ever way you look at it, is considerable backward compatibility will be lost! And a small word about And while I can see the new nomenclature of 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 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 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... |
@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. |
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:
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:
Here's the current state of options that affect output:
TidyInfo
messages (only)TidyWarning
messages (only)tidyRunDiagnostics
to squelchTY_(ReportMarkupVersion)( doc )
and TY_(ReportNumWarnings)( doc ). We should classify these messages as a different report level, and worry about squelching them. Used in triggeringtidyErrorSummary
(the footnotes) andtidyGeneralInfo.
TidyInfo
, so why not just count onTidyInfo
?Proposed replacement:
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:
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:
The text was updated successfully, but these errors were encountered: