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

Improve output of multi-line stderr from local command #6282

Merged
merged 2 commits into from Dec 11, 2023

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Dec 9, 2023

Background

We have a few local commands that run from the Tiltfile before starting resources. We use these commands to output kubernetes yaml that is passed to k8s_yaml. These commands have rather verbose stdout (thousands of lines of yaml), so it's important for us to run these with quiet = True. When one of these commands fail, the stderr is also relatively verbose, it will contain multiple lines to help the user understand why it failed and where the problem occurred.

Problem

We've noticed that it can be difficult to read these multi-line error messages because they are printed using %q. This escapes backslashes and replaces newlines with \n. For example:

stderr: "\nFailed to do some thing:\n at src/myproject/dir/file:2:40\n - reason one\n - reason two\nAlso failed this other thing\n at src/myproject/dir/other:2:40\n - more reasons\n"

Changes in this PR

This PR changes the format string to use %v for this output so that it reads better.

Error in local: command "./script.sh" failed.
error: exit status 1
stdout:
...
stderr:
Failed to do some thing:
 at src/myproject/dir/file:2:40
 - reason one
 - reason two
Also failed this other thing
 at src/myproject/dir/other:2:40
 - more reasons

Does this seem reasonable? Any concerns about where this wouldn't work well?

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
internal/tiltfile/files.go Outdated Show resolved Hide resolved
Co-authored-by: Daniel Nephin <dnephin@gmail.com>
@nicks
Copy link
Member

nicks commented Dec 11, 2023

ya, i'm fine with going with the more simple approach and seeing if anyone complains

@nicks nicks self-requested a review December 11, 2023 15:15
@nicks nicks merged commit 24fdc0c into tilt-dev:master Dec 11, 2023
8 of 9 checks passed
@nicks
Copy link
Member

nicks commented Dec 11, 2023

LGTM

@dnephin dnephin deleted the localcmd-stderr branch December 11, 2023 21:25
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.

None yet

2 participants