tsuyoshicho / action-mypy

Run mypy with reviewdog on pull requests to improve code writing experience.
Creative Commons Zero v1.0 Universal
20 stars 8 forks source link

Add no pretty try tee #118

Closed bernhardkaindl closed 9 months ago

bernhardkaindl commented 10 months ago

I think I found the issue why my mypy messages in #115 are truncated very early:

I've pretty = true in my existing mypy configuration for nicer output to the terminal.

But when script.sh pipes the mypy output to reviewdog, the enabled pretty option causes mypy to wrap the output at column 79 (see the examples below):

mypy with pretty = true in the configuration file, with output directly to the terminal's tty:

mypy scripts/mail-alarm
scripts/mail-alarm: note: In function "__init__":
scripts/mail-alarm:726: error: Item "None" of "Optional[Node]" has no attribute "toxml"  [union-attr]
                    return xmldoc.getElementsByTagName(tag)[0].firstChild.toxml()
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

mypy with pretty = true in the configuration file, with piped to a command like for reviewdog in script.sh:

mypy --show-absolute-path scripts/mail-alarm |head -5
/home/user/git/github/master/prs/xen-api/scripts/mail-alarm: note: In function "__init__":
/home/user/git/github/master/prs/xen-api/scripts/mail-alarm:726: error: Item
"None" of "Optional[Node]" has no attribute "toxml"  [union-attr]
                    return xmldoc.getElementsByTagName(tag)[0].firstChild....
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~...

Of course, the truncation is even worse with the --show-absolute-path that script.sh adds.

Is the --show-absolute-path really needed?

Here, adding --no-pretty helps to get the expected non-truncated error message in a single line:

mypy --no-pretty scripts/mail-alarm |head -2
scripts/mail-alarm: note: In function "__init__":
scripts/mail-alarm:726: error: Item "None" of "Optional[Node]" has no attribute "toxml"  [union-attr]

Not knowing that --pretty causes this, in my experiments to diagnose it, I had also tried adding --pretty to action-mypy.with.mypy_flags, which of course we should override to prevent users from messing up the output format of mypy that the action relies on.

INPUT_MYPY_FLAGS=--pretty  # Bad, we need to override it:
mypy ${INPUT_MYPY_FLAGS} --no-pretty scripts/mail-alarm |head -2
scripts/mail-alarm: note: In function "__init__":
scripts/mail-alarm:726: error: Item "None" of "Optional[Node]" has no attribute "toxml"  [union-attr]

Good, overriding incompatible mypy flags should work just fine this way.

Thus, move ${INPUT_MYPY_FLAGS} to be first on the command line and override any incompatible flags from action-mypy.with.mypy_flags with them: --no-pretty, --show-column-numbers and --show-absolute-path.

I hope that this fixes the formatting issues - to be tested.

In the 2nd commit I also attempt to use reviewdog -tee to hopefully cause reviewdog to send mypy warnings to stdout as well get them into the log of the GitHub action workflow as well.

Related issue

related to #115

Fix or Add/Remove in this PR:

Test/動作確認

With one bigger commit to fix installing types (needed after set -euvx was added) it looks good now (the new comments at the end are good):

https://github.com/xenserver-next/xen-api/pull/6

tsuyoshicho commented 9 months ago

Hi @bernhardkaindl san,

Regarding your suggestion of pretty print suppression, we have created a PR that implements the core part here.

I'll close this PR once. However, if you think it would be better to make additional modifications, please re-create the PR with the modified content after applying the above PR.

This PR was well done, the content is not to be denied.