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

pkg/columns: Do not elide output when redirected. #2451

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

eiffel-fl
Copy link
Member

Hi.

This PR removes ellipsis when the output is not a terminal:

$ sudo -E ./ig image list | awk '{ printf "%s:%s\n", $1, $2 }' | tail -2
INFO[0000] Experimental features enabled                
blocks:latest
ghcr.io/inspektor-gadget/gadgets/blocks:latest

Best regards.

@eiffel-fl eiffel-fl linked an issue Feb 5, 2024 that may be closed by this pull request
@eiffel-fl eiffel-fl marked this pull request as ready for review February 5, 2024 08:12
@eiffel-fl eiffel-fl requested a review from flyth as a code owner February 5, 2024 08:12
@mqasimsarfraz
Copy link
Member

Thanks for working on this, it will be nice to have this.

Also, what do you think about combining it with --no-trunc ?

@flyth
Copy link
Member

flyth commented Feb 5, 2024

Maybe this should rather become an Option.

Otherwise we lose the possibility of formatting output for other targets (e.g. when used as a service and sending formatted output to another logserver or such), because only the local environment would be considered.

@eiffel-fl eiffel-fl force-pushed the francis/no-elipsis-redirect branch 2 times, most recently from b991ec7 to c266274 Compare February 6, 2024 07:36
@eiffel-fl
Copy link
Member Author

I just added the Option and dealt also with noTrunc:

$ sudo -E ./ig image list | awk '{ printf "%s:%s\n", $1, $2 }' | tail -2
INFO[0000] Experimental features enabled                
blocks:latest
ghcr.io/inspektor-gadget/gadgets/blocks:latest
$ sudo -E ./ig image list | awk '{ printf "%s:%s\n", $1, $3 }' | tail -2
INFO[0000] Experimental features enabled                
blocks:sha256:2bea84b83af28cd47302dcf37fe115327637974c9db4d490b5a59c65df4c190c
ghcr.io/inspektor-gadget/gadgets/blocks:sha256:2bea84b83af28cd47302dcf37fe115327637974c9db4d490b5a59c65df4c190c

Copy link
Member

@flyth flyth left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! I'd just rename the field + option as it now just decides whether or not to shorten/truncate the string, not whether an ellipsis is used.

pkg/columns/formatter/textcolumns/options.go Outdated Show resolved Hide resolved
When used in Inspektor Gadget, output will not be truncated if it is not a
terminal.

Signed-off-by: Francis Laniel <flaniel@linux.microsoft.com>
Copy link
Member

@flyth flyth left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@eiffel-fl
Copy link
Member Author

Thank you for the review!

@eiffel-fl eiffel-fl merged commit 7cc6999 into main Feb 6, 2024
56 checks passed
@eiffel-fl eiffel-fl deleted the francis/no-elipsis-redirect branch February 6, 2024 09:21
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 has ellipsises when redirected
3 participants