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

Ormolu script improvements #1458

Merged
merged 2 commits into from
Apr 19, 2021
Merged

Ormolu script improvements #1458

merged 2 commits into from
Apr 19, 2021

Conversation

pcapriotti
Copy link
Contributor

  • Reduce size of ormolu output when running in a terminal

  • Make the ormolu script detect Ctrl-C more reliably

It seems that ormolu returns exit code 0 on SIGTERM, and this causes the ormolu
script to not react to Ctrl-C, if the ormolu process happens to receive it (as
opposed to the rest of the shell script).

To fix this, run ormolu in background, and wait on the corresponding process.
This makes the wait call handle the interrupt, so that we can distinguish
between normal failure (exit code 100), success (exit code 0) and abnormal
failure.
@pcapriotti pcapriotti merged commit 9226082 into develop Apr 19, 2021
@pcapriotti pcapriotti deleted the pcapriotti/condense-ormolu branch April 19, 2021 14:26
@@ -72,14 +72,26 @@ echo "language extensions: $LANGUAGE_EXTS"

FAILURES=0

if [ -t 1 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

why this trivial conditional?

anyway i like the changes!

Copy link
Contributor Author

@pcapriotti pcapriotti Apr 26, 2021

Choose a reason for hiding this comment

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

[ -t 1 ] returns 0 if the standard output is a tty. This check is to prevent the script from printing escape sequences when it runs in CI, or any other environment which is not a shell, basically.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, my bad: I read -t as isTrue. Checking the man page would have helped. :)

This was referenced May 4, 2021
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.

3 participants