feat: Add option quiet to hide warnings in output #629
Conversation
|
had some trouble with |
|
Thanks a lot for your contribution @marianfoo. We will take care of this PR in backlog item CPOUI5FOUNDATION-837. |
| fixableWarningCount: 0, | ||
| })); | ||
| }; | ||
|
|
There was a problem hiding this comment.
Why not overwrite res here once so it affects all the cases below?
There was a problem hiding this comment.
you´re right, makes more sense, changed that
src/formatter/json.ts
Outdated
| @@ -28,7 +28,7 @@ export class Json { | |||
| filePath: oLintedFile.filePath, | |||
| messages: aFileMessages, | |||
| errorCount: oLintedFile.errorCount, | |||
| warningCount: oLintedFile.warningCount, | |||
| warningCount: quiet ? 0 : oLintedFile.warningCount, | |||
There was a problem hiding this comment.
Is this necessary? warningCount should be set to 0 in the filter function?
There was a problem hiding this comment.
you´re right, i think i did that before i implemented to filter the messages, changed it
src/formatter/markdown.ts
Outdated
| if (quiet) { | ||
| summary += | ||
| `> ${totalErrorCount} ${totalErrorCount === 1 ? "problem" : "problems"} ` + | ||
| `(${totalErrorCount} ${totalErrorCount === 1 ? "error" : "errors"}) \n`; | ||
| } else { | ||
| summary += | ||
| `> ${totalErrorCount + totalWarningCount} problems ` + | ||
| `(${totalErrorCount} errors, ${totalWarningCount} warnings) \n`; | ||
| } |
There was a problem hiding this comment.
Ideally both outputs behave the same. So If we make a distinction for singular/plural in quite mode, I would expect the same in regular mode.
Maybe something like this?
| if (quiet) { | |
| summary += | |
| `> ${totalErrorCount} ${totalErrorCount === 1 ? "problem" : "problems"} ` + | |
| `(${totalErrorCount} ${totalErrorCount === 1 ? "error" : "errors"}) \n`; | |
| } else { | |
| summary += | |
| `> ${totalErrorCount + totalWarningCount} problems ` + | |
| `(${totalErrorCount} errors, ${totalWarningCount} warnings) \n`; | |
| } | |
| const errorsText = `${totalErrorCount} ${totalErrorCount === 1 ? "error" : "errors"}`; | |
| let warningsText = ""; | |
| if (!quiet) { | |
| warningsText = `, ${totalWarningCount} ${totalWarningCount === 1 ? "warning" : "warnings"}`; | |
| } | |
| summary += | |
| `> ${totalErrorCount + totalWarningCount} problems ` + | |
| `(${errorsText}${warningsText}) \n`; |
There was a problem hiding this comment.
makes sense, implemented
src/formatter/text.ts
Outdated
| if (quiet) { | ||
| this.#writeln( | ||
| summaryColor( | ||
| `${totalErrorCount} ${totalErrorCount === 1 ? "problem" : "problems"} ` + |
test/lib/cli/base.integration.ts
Outdated
|
|
||
| t.is(consoleLogStub.callCount, 0, "console.log should not be used"); | ||
| t.true(processCwdStub.callCount > 0, "process.cwd was called"); | ||
| t.is(processStdoutWriteStub.callCount, 2, "process.stdout.write was called twice"); | ||
| t.is(processExitStub.callCount, 0, "process.exit got never called"); | ||
| t.is(process.exitCode, 1, "process.exitCode was set to 1"); |
There was a problem hiding this comment.
What was the problem with the old exitCode handling in this test?
There was a problem hiding this comment.
i tried thing to make the test work but it always falis in gh action, dont know the reason, i reverted those changes
marianfoo
force-pushed
the
feat/option-quiet
branch
from
521fcbc to
b10be3d
Compare
Thank you for your contribution! 🙌
To get it merged faster, kindly review the checklist below:
Pull Request Checklist