Skip to content

Conversation

evanelias
Copy link
Contributor

Linter annotation messages sometimes contain embedded newlines, which can make automated parsing of these messages difficult. This PR adjusts linter output to strip embedded newlines whenever STDERR isn't a terminal. In this situation, spaces are now used in place of newlines, so each linter annotation is guaranteed to be a single log line.

Note that non-linter-related log messages may still contain embedded newlines. In that situation, if STDERR isn't a terminal, each log line repeats the log message header with timestamp and severity.

The logic in this PR only pays attention to STDERR (and not STDOUT) since Skeema log messages always go to STDERR. In skeema diff and skeema push, STDOUT is used for emitting SQL. In skeema lint, STDOUT is currently unused.

This PR also adds some utility functions for making it easier to check whether a file descriptor is a terminal.

Linter annotation messages sometimes contain embedded newlines, which can
make automated parsing of these messages difficult. This commit adjusts
linter output to strip embedded newlines whenever STDERR isn't a terminal. In
this situation, spaces are now used in place of newlines, and each linter
annotation is guaranteed to be a single log line.

Note that non-linter-related log messages may still contain embedded
newlines.

The logic in this commit only pays attention to STDERR (and not STDOUT) since
Skeema log messages always go to STDERR. In `skeema diff` and `skeema push`,
STDOUT is used for emitting SQL. In `skeema lint`, STDOUT is currently unused.

This commit also adds some utility functions for making it easier to check
whether a file descriptor is a terminal.
@coveralls-official
Copy link

Coverage Status

Coverage: 93.327% (+0.01%) from 93.312% when pulling 332c36c on non-tty-linter-annotations into c15bee1 on main.

@evanelias evanelias merged commit 93ad4de into main Apr 19, 2023
@evanelias evanelias deleted the non-tty-linter-annotations branch April 19, 2023 19:51
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.

1 participant