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

feat: add short table output #1292

Merged
merged 2 commits into from
Aug 9, 2023
Merged

Conversation

XDRAGON2002
Copy link
Contributor

@XDRAGON2002 XDRAGON2002 commented Jul 22, 2023

Overview

As mentioned in the issue, the extremely wide tabular output doesn't fit in smaller terminals in a representable manner, hence a smaller more compact tabular format has been added which automatically infers based on the given terminal if the wider output needs to be rendered or the smaller.

Additional Information

In order to detect the terminal size a new dependency has been used, and automatically displays the smaller output tables if the terminal width is not able to support the original table output.
It does so by pivoting the table hence decreasing the width but instead increasing the height. A tradeoff made depending on the available width.

How to Test

On a smaller terminal screen with width less than 120, run any of:

  1. kubescape list controls
  2. kubescape scan resource.yaml
  3. kubescape scan resource.yaml -v

Examples/Screenshots

Before:

image
image
image

After:

image
image
image

Related issues/PRs:

Resolved #933

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have commented on my code, particularly in hard-to-understand areas
  • I have performed a self-review of my code
  • If it is a core feature, I have added thorough tests.
  • New and existing unit tests pass locally with my changes

@XDRAGON2002
Copy link
Contributor Author

@craigbox I've added the ability to automatically detect the terminal width, and formatted the table as discussed.
Do let me know if there are any further changes needed!
Thanks!

@XDRAGON2002
Copy link
Contributor Author

@craigbox I've made the required changes and now the table format is decided based on whether the large output tables are able to be displayed on the current terminal or not, if not then the smaller table is displayed ;)

@craigbox
Copy link
Contributor

wonderful. to @dwertent & @matthyx for review please!

@craigbox
Copy link
Contributor

craigbox commented Aug 1, 2023

Can you please fix the conflicts, and we can get this merged soon I hope.

@XDRAGON2002
Copy link
Contributor Author

@craigbox done!

@dwertent
Copy link
Contributor

dwertent commented Aug 4, 2023

@XDRAGON2002 I see there are still conflicts.
Probably because we merged #1320 .

Signed-off-by: DRAGON <anantvijay3@gmail.com>
@ghost
Copy link

ghost commented Aug 9, 2023

👇 Click on the image for a new way to code review

Review these changes using an interactive CodeSee Map

Legend

CodeSee Map legend

@dwertent
Copy link
Contributor

dwertent commented Aug 9, 2023

@XDRAGON2002 Here is the issue.

I recommend you run before you push the following commands:

go test "-tags=static,gitenabled" -v ./...

and

make build

Signed-off-by: DRAGON2002 <81813720+XDRAGON2002@users.noreply.github.com>
Signed-off-by: DRAGON <anantvijay3@gmail.com>
@XDRAGON2002
Copy link
Contributor Author

@XDRAGON2002 Here is the issue.

I recommend you run before you push the following commands:

go test "-tags=static,gitenabled" -v ./...

and

make build

Sorry for that! I had actually used the GitHub web UI to fix merge conflicts which is where I messed up a variable! Thanks for that!

I'll make sure to get in the habit of ensuring tests and builds before pushing.

The only reason I forget to do that every now and then is because I keep forgetting we do not have a pre-commit-hook configured. Yet 👀, hopefully we're able to get this in.

@dwertent
Copy link
Contributor

dwertent commented Aug 9, 2023

It works as expected, I will merge it as soon as the tests pass :)

@XDRAGON2002
Copy link
Contributor Author

@dwertent this one is good to go now.

@dwertent dwertent merged commit 6c1a3fb into kubescape:master Aug 9, 2023
7 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.

Output table does not fit in my terminal
3 participants