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

Only process when there is output to return #116

Merged
merged 3 commits into from
Aug 21, 2022

Conversation

elken
Copy link
Contributor

@elken elken commented Aug 17, 2022

scalafmt only returns stdout when there are errors, this could also help
in other cases where a formatter returns incorrectly.

Ideally, this would also be improved by basic error checking heuristics
to work out if we need to update the buffer.

elken and others added 2 commits August 17, 2022 07:27
scalafmt only returns stdout when there are errors, this could also help
in other cases where a formatter returns incorrectly.

Ideally, this would also be improved by basic error checking heuristics
to work out if we need to update the buffer.
Copy link
Member

@raxod502 raxod502 left a comment

Choose a reason for hiding this comment

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

✨ Thanks. I updated the changelog accordingly. In my opinion, if a formatter does this then it is a bug that should be fixed upstream, but it is undeniable that the change you've suggested here is a simple way to improve the user experience in Apheleia irrespective of such bugs.

@raxod502 raxod502 merged commit 04366a9 into radian-software:main Aug 21, 2022
@elken elken deleted the fix/empty-formatters branch August 22, 2022 05:37
@elken
Copy link
Contributor Author

elken commented Aug 22, 2022

✨ Thanks. I updated the changelog accordingly. In my opinion, if a formatter does this then it is a bug that should be fixed upstream, but it is undeniable that the change you've suggested here is a simple way to improve the user experience in Apheleia irrespective of such bugs.

In the case of scalafmt it seems to be by design rather than a bug 😅

But yeah I do agree

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

Successfully merging this pull request may close these issues.

2 participants