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

Trim overlong source fragments in error messages #4885

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

ChrisDodd
Copy link
Contributor

  • too-long source lines (particularly those that come from preprocessor macros) tend to give unreadble error messages. Ideally they should be trimmed based on the width of the output terminal.

This code is a compromise that defaults the trim width to 100 when the useMarker flag is defaulted to true (the way this is used for user-facing error messages) and to 0 (no trimming) when the flag is set explicit (generally to false) -- one place this is used is in some logging code.

Copy link
Contributor

@vlstill vlstill left a comment

Choose a reason for hiding this comment

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

Nice addition to make the messages easier to read, thank you!

Alternative to hadrcoded 100 would be to default to the current terminal width (with a fallback to 100 or 80), but I guess detecting that every time a source info is being formatted would not be practical.

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Aug 27, 2024
@ChrisDodd
Copy link
Contributor Author

Alternative to hardcoded 100 would be to default to the current terminal width (with a fallback to 100 or 80), but I guess detecting that every time a source info is being formatted would not be practical.

If we default to the terminal width, we'd want a way to override that with a command line arg (and use that arg in ctest) -- otherwise you'd see spurious test failures due to using different terminal sizes. There's also the question of how to portably determine terminal size -- linux/windows are quite different and we've so far avoided putting OS-specific stuff in the compiler. Best compromise would probably be to use the COLUMNS environment variable, which both can use (and we could override in run_test.py).

- too-long source lines (particularly those that come from preprocessor
  macros) tend to give unreadble error messages.  Ideally they should be
  trimmed based on the width of the output terminal.

Signed-off-by: Chris Dodd <[email protected]>
@ChrisDodd ChrisDodd added this pull request to the merge queue Aug 28, 2024
Merged via the queue into p4lang:main with commit aa9cbc1 Aug 28, 2024
18 checks passed
@ChrisDodd ChrisDodd deleted the cdodd-errfrag-trim branch August 28, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants