-
Notifications
You must be signed in to change notification settings - Fork 30
Feat: Add empty json output mode #844
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
base: main
Are you sure you want to change the base?
Feat: Add empty json output mode #844
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR! The {}
should be only printed for the JSON printer, for the normal printer there's no point in printing an additional empty line.
Actually, it should be easy to solve this fully generally for all commands with this approach:
- Remember in the JSON printer if anything was printed.
- Add a
finalize_output
function to the printer, which will be called just before HQ exits (even if an error has occurred). - Implement
finalize_output
for the JSON printer in a way where it will output{}
if nothing was previously printed.
my apologies for the late reply, I am currently writing exams, I will be done next week and pick this up. Thanks! |
…nto feat/empty-json-output-mode
Signed-off-by: tdadadavid <[email protected]>
@@ -251,6 +251,10 @@ impl Output for JsonOutput { | |||
fn print_error(&self, error: Error) { | |||
self.print(json!({ "error": format!("{error:?}") })) | |||
} | |||
|
|||
fn finalize_output(&self) { | |||
self.print(json!({})); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should only be printed if we did not print anything in the JSON printer. Otherwise we will print multiple JSON objects, which is invalid JSON.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh thanks I will work on this now.
@@ -1034,6 +1034,10 @@ impl Output for CliOutput { | |||
job_id.to_string().color(colored::Color::Green), | |||
); | |||
} | |||
|
|||
fn finalize_output(&self) { | |||
println!() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to print anything here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks will fix this.
[✅] Add
print_empty
to theOutput
trait[✅] Implemented
print_empty
forJSON
,Cli
andQuiet
For #830