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

Add option to hide build command on compilation error to build_runner #8513

Merged
merged 4 commits into from Jul 28, 2021
Merged

Add option to hide build command on compilation error to build_runner #8513

merged 4 commits into from Jul 28, 2021

Conversation

leecannon
Copy link
Contributor

@leecannon leecannon commented Apr 12, 2021

I know #6673 is still open with no consensus for the long term improvement of cli errors, but with a project with a large number of C files added you get errors like this (~36,000 characters of build command) which is just actively hostile to a nice debugging experience.

This PR is a stopgap that is not intended to be permanent which adds hide_build_command_on_error: bool on builder and --hide-build-command-on-error command line argument when using zig build

The scope of this change is kept small with the flag only taken into account in execFromStep when encountering a ExitCodeFailure as this is the path that a normal failed compilation will execute, rather than checking this flag everytime a failed command is to be printed e.g. on file not found.

@andrewrk
Copy link
Member

Rather than --hide-build-command-on-error I suggest for this feature to be named after the actual goal: surface compile errors. The implementation can be the same as this stop gap solution for now, but it allows room to improve the feature to do the following:

  • Detect compile errors and present only them
  • Still print the build command as usual if a non-compile-error crash happened

Counter proposal: --prominent-compile-errors
Description: "Indicate the purpose of executing the command is for a human to read compile errors from the terminal"

In the future there will be other options that make the output more consumable by an IDE and other tooling, so we want to be sure to make clear the niche this flag serves.

@leecannon
Copy link
Contributor Author

Does it make sense for it to be the other way round?

i.e. zig assumes without any additional flags that its output is for human consumption and requires additional flags for more detailed output for use by tooling, compiler troubleshooting, etc.

@andrewrk
Copy link
Member

Description: "Indicate the purpose of executing the command is for a human to read compile errors from the terminal"

There are 2 components here, not just 1:

  • human, not computer, is the agent
  • the intended purpose is to read compile errors

The default use case is:

  • if detected a TTY, then it is a human, otherwise it is machine
  • intended purpose is to silently succeed the build, or loudly fail with all the information needed to troubleshoot a problem anywhere on the process stack.

@leecannon
Copy link
Contributor Author

leecannon commented Jun 22, 2021

Im not 100% happy with the description of the flag in the usage text

--prominent-compile-errors   Output compile errors formatted for a human to read

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants