--mute should suppress non-zero exit code #794
Comments
|
@yaccz thanks for the issue... It is also good to give a minimum html example input, tidy version, config used, output, and expected output, just to make quick testing easier... Here I built some of my own, 4 My first sample - <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Is. #754</title>
</head>
<body>
<p>Drop empty element</p>
<p></p>
<p>--mute TRIM_EMPTY_ELEMENT should drop warning</p>
</body>
</html>At first I thought this might be There are several precedents where an option avoids a Off the top of my head is say, a But there are other examples of where the users input changes the So if the user configures At present, on sample 1, with It says Although, in this case, there is a Seems Look forward to comments, patches, PR to change this behavior, if all agreed this is a valid Feature Request... Thinking, testing, trying... thanks... |
yeah, sorry about that. I felt like this case is simple enough to skip those.
I am not familiar with the tidy internals but your analysis seems correct to me. Though I'd classify this a Bug rather than Feature Request given the inconsistency of reported warning count vs printed warnings. |
|
@yaccz have started to look a little deeper... thanks for the feedback... The classification is not important... if the maintainer, who introduced this Which is Maybe my too quick choice of In further testing, find there are others cases, where the A quick example - <!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Is. #754-4</title>
</head>
<body>
<p>Open div warning, suppressed</p>
<div id="open">
<p>'--show-warnings no' drops msg but not exit-level</p>
</body>
</html>The test output - Again But does have the addition Maybe that could also be triggered by a So we can see In this case Still digging... comments, feedback welcome... thanks... |
This might be a good use case for suppressed |
|
@ler762 thanks for the additional testing... but not sure what has been added...
The answer to this is current simple code choice - some spaces removed for compactness - Not very exact, but sort of does the job... and Reading the API docs on mute... it seems clear... it has the words It says nothing about also preventing Tidy bumping doc warnings... that is the exit code... and thinking about it more, why should it... the exit code is the true state, and must be kept/maintained, IMO... While it seems/is ok to suppress console message output... we all want that!... I implemented But never changing the Also See the code order of messageOut... note the All in all, it seems it is not ok to change the The question becomes, should If yes, give the use case... and for all, or some... be specific, as best you can... This does not seem like one of them... at least at the moment... and maybe should be a new separate issue anyway... @yaccz, as suggested earlier, you could add The default We should support the Appreciate further comments, feedback, patches, even tested PR, to implement, at least this step... thanks... |
|
@yaccz - Testing patch to add diff --git a/src/message.c b/src/message.c
index 0900407..198ca2e 100644
--- a/src/message.c
+++ b/src/message.c
@@ -1291,12 +1291,19 @@ void TY_(ReportMarkupVersion)( TidyDocImpl* doc )
/* Reports the number of warnings and errors found in the document.
** Called by tidyRunDiagnostics(), from console.
+**
+** If there are 'warnings' or 'errors', then add 'not all shown'
+** msg, if the are any options that 'limit' message output. Not
+** intended to be accurate or exact, just an indication that
+** some message(s) **may** have been suppressed, for some reason.
*/
void TY_(ReportNumWarnings)( TidyDocImpl* doc )
{
if ( doc->warnings > 0 || doc->errors > 0 )
{
- if ( doc->errors > cfg(doc, TidyShowErrors) || !cfgBool(doc, TidyShowWarnings) )
+ if ( doc->errors > cfg(doc, TidyShowErrors) || !cfgBool(doc, TidyShowWarnings) ||
+ doc->muted.list) /* Is. #794 - add '--mute XXXX' option
+ to other options which limit messages */
{
TY_(Dialogue)( doc, STRING_NOT_ALL_SHOWN );
}Seems to work... Appreciate if others could test, confirm this fixes the conformity bug... thanks... |
|
To make it easy for testing, have pushed this patch to the This change may effect the tidy-html4-tests regression tests... still to test this... any help appreciated... As stated, be great if others could test, and confirm this patch/fix... and will get to a PR... thanks... |
|
In a quick |
|
I am sorry, I am quite busy at the moment and this issue came up on a side project of mine. I should be able to test in a week or two. |
|
Not working as expected. Reproducer: Lines like Indicate that the previous command ended with exit code 1 by the ending on |
|
@yaccz thank you for testing I assume you Please re-read all the comments, except my first! ;=)) At first I thought it a reasonable request, but on exploring, testing more, I rejected this... reversed my initial thoughts... sorry... Citing that there were already some options to suppress message output, including Thus the only difference between current to But both will exit with 1... this has not changed... The document has been modified by It is not Note to self, Look forward to further feedback... thanks... |
|
Ok, then it looks like it works as you intended. |
|
re-opening this it would be really good if tidy would not adjust the error code for warnings consider error code is 1. while it's great that it's not kind of "writing" the warning to console; i'm actually using tidy as part of a toolchain, and when the exit code isn't 0, the chain fails. so, how can i just ask tidy to suppress this warning? |
|
@silky, thank you for the comment, and sorry for the LONG delay in replying on this... There is simply NO option to modify the exit code... The Please re-read my comments, particularly -
For the particular case given in this issue, there exists an option, In the circumstances where tidy is being used in a So, at this point, can see no reason to re-open this... sorry... |
I am running tidy as part of static web site build pipeline and as a workaround for an issue with syntax highlighter I need to ignore TRIM_EMPTY_ELEMENT warning.
Note this is the only warning I get. When I mute it, it does not show in the output anymore, but it still causes non-zero exit code which breaks my build.
I find this behavior quite unintuitive and think --mute should also mute changes to exit code.
The text was updated successfully, but these errors were encountered: