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

fix: Do not print extraneous newlines when executionInfo output is hidden #519

Merged
merged 4 commits into from
Jul 18, 2023
Merged

fix: Do not print extraneous newlines when executionInfo output is hidden #519

merged 4 commits into from
Jul 18, 2023

Conversation

hyperupcall
Copy link
Contributor

Closes #494

⚡ Summary

Prevents printing blank lines when exececutionInfo is not printed.

Previously, two newlines would be printed for each command (Effectively fmt.Println("\n")). This fixes that.

☑️ Checklist

  • Check locally
  • Add tests

Copy link
Member

@mrexox mrexox left a comment

Choose a reason for hiding this comment

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

Overall looks good, just a small nitpick.

Comment on lines +488 to +492
if execLog != "" {
log.Info(execLog)
}

if err == nil && r.SkipSettings.SkipExecutionOutput() {
Copy link
Member

@mrexox mrexox Jul 17, 2023

Choose a reason for hiding this comment

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

The check for execution skipping should go first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

So I guess this means that the previous log.Info(execLog) was not intended to be there - Because it seems that now the behavior just changed so that execLog is never printed when the condition err == nil && r.SkipSettings.SkipExecutionOutput() is met.

Copy link
Member

Choose a reason for hiding this comment

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

Oh god, I'm sorry. execLog shouldn't be printed if it is empty. That's right. And if we have SkipExecutionOutput() then we don't print command output. So, your previous solution was correct. Let me fix that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Haha yeah I thought something was fishy - glad it's sorted out now 😅

@mrexox mrexox merged commit e227a43 into evilmartians:master Jul 18, 2023
@hyperupcall hyperupcall deleted the h-improve-execution-output branch July 18, 2023 07:58
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.

Compress the blank lines printed
2 participants