Skip to content

sbctl: Fix human readable output being printed when using --json #342

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

Merged
merged 1 commit into from
Aug 2, 2024
Merged

sbctl: Fix human readable output being printed when using --json #342

merged 1 commit into from
Aug 2, 2024

Conversation

chenxiaolong
Copy link
Contributor

@chenxiaolong chenxiaolong commented Jul 31, 2024

PersistentPreRun was being overwritten later in main(), causing logging.PrintOff() to never be called.

EDIT: Updated with @Foxboron's suggestion below.

@Foxboron
Copy link
Owner

Foxboron commented Aug 1, 2024

We should be setting a global on variable in the logging path to toggle printing and no printing.

It's unclear to me why this doesn't work.

https://github.com/Foxboron/sbctl/blob/master/logging/logging.go#L50

@Foxboron
Copy link
Owner

Foxboron commented Aug 1, 2024

Ah, I found the issue. I overwrite PersistentPreRun in the main function. If you could move the code here https://github.com/Foxboron/sbctl/blob/master/cmd/sbctl/main.go#L68 to here https://github.com/Foxboron/sbctl/blob/master/cmd/sbctl/main.go#L136 that would fix the issue :)

@chenxiaolong
Copy link
Contributor Author

chenxiaolong commented Aug 1, 2024

Ah, I totally missed that the logging system itself had a flag for this. Thanks! I'll update the PR with your suggestion.

`PersistentPreRun` was being overwritten later in `main()`, causing
`logging.PrintOff()` to never be called.

Signed-off-by: Andrew Gunnerson <[email protected]>
@chenxiaolong chenxiaolong changed the title sbctl: Don't print human readable output when using --json sbctl: Fix human readable output being printed when using --json Aug 1, 2024
@chenxiaolong
Copy link
Contributor Author

Done!

@Foxboron
Copy link
Owner

Foxboron commented Aug 2, 2024

Thanks!

@Foxboron Foxboron merged commit 3aaf001 into Foxboron:master Aug 2, 2024
@Foxboron
Copy link
Owner

Foxboron commented Aug 2, 2024

Thanks!

@chenxiaolong chenxiaolong deleted the list-files branch August 2, 2024 20:18
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.

2 participants