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

Format powershell linter output into terminal-wide table #3318

Merged
merged 14 commits into from Feb 8, 2024

Conversation

efrecon
Copy link
Contributor

@efrecon efrecon commented Jan 24, 2024

Fixes #3313 and #3320

Proposed Changes

Improve the output format of the powershell linter by ensuring that the columns utilise the entire terminal width. This improves readability of both the description message and the location of the offending problem. Extra care is necessary to bypass a known powershell bug on UNIX-like systems.

Readiness Checklist

Author/Contributor

  • Add entry to the CHANGELOG listing the change and linking to the corresponding issue (if appropriate)
  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

efrecon and others added 2 commits January 24, 2024 14:36
Query the width of the terminal and enforce it for the output of the
powershell linter. This uses the following cmdlets:

- Format-Table is used for a better table output, made aware of the
  extra width available.
- Out-String is explicity set to use the width of the terminal coming
  from Python.
@efrecon
Copy link
Contributor Author

efrecon commented Jan 24, 2024

I see that the tests fail. This is because the output has changed and is not clamped to 80 characters anymore.

An "easy" solution would be to arrange for the content of the ERROR-POWERSHELL_POWERSHELL.txt file to be the same as the one when running the tests at GitHub. Where can I find the output in the GitHub workflow logs/files?

A better solution, i.e. working in all cases, would require to add a new environment variable called POWERSHELL_POWERSHELL_OUTPUT_WIDTH (or similar) and to:

  1. Control and enforce the content of that variable during the tests.
  2. Arrange for the output file to match (or don't change and enforce at 80)

Which solution is best and how can we enforce the content of an environment variable during a test?

@nvuillam
Copy link
Member

nvuillam commented Jan 27, 2024

@efrecon The error seems related to the fact that the terminal is not tty (basically means that it is not interactive)

        if self.linter_name == "powershell":
            # Format output to fit in terminal, respecting the terminal width.
            # For more info: https://stackoverflow.com/a/76889369
>           size = os.get_terminal_size()
E           OSError: [Errno 25] Not a tty

You should implement a fallback with a default size, or even just define a default size, as terminal will never be interactive, and it should work :)

efrecon and others added 2 commits January 29, 2024 17:29
It has defaults when no TTY present, same as PowerShell
efrecon and others added 5 commits January 29, 2024 20:35
This count the lines beginning with all known severity errors
…ter into better-powershell-output

* 'better-powershell-output' of github.com:efrecon/megalinter:
  [automation] Auto-update linters version, help and documentation (oxsecurity#3338)
  Bump peter-evans/create-pull-request from 5 to 6 (oxsecurity#3337)
  Bump yeoman-environment from 4.2.1 to 4.3.0 in /mega-linter-runner (oxsecurity#3333)
  [automation] Auto-update linters version, help and documentation (oxsecurity#3334)
  [automation] Auto-update linters version, help and documentation (oxsecurity#3331)
@efrecon
Copy link
Contributor Author

efrecon commented Feb 5, 2024

Now integrates fixes for #3320 as well

@nvuillam
Copy link
Member

nvuillam commented Feb 7, 2024

sorry for the delay, i'm very busy these day ^^
Thanks for the new commits, let's hope CI jobs will pass :)

efrecon and others added 2 commits February 8, 2024 05:58
Each part of the pipeline is documented to facilitate understanding
Copy link
Member

@nvuillam nvuillam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks a lot @efrecon for your hardwork on this PR :)

@nvuillam nvuillam merged commit ad6aaa7 into oxsecurity:main Feb 8, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve output format of the powershell linter
2 participants