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

Print a usage description when the user enters the swift format command without arguments #914

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

TTOzzi
Copy link
Contributor

@TTOzzi TTOzzi commented Jan 11, 2025

As discussed in #871 and in this forum thread Default behavior of “swift format”, waiting for stdin when only the swift format command is entered causes significant confusion for new users.
Although there were suggestions to provide a default behavior for convenience, I agree that it is not appropriate for a destructive change to take effect immediately.

However, at the very least, some guidance is needed to prevent the command from being perceived as hanging, so I've improved usability to output additional guidance when the user enters the swift format command alone.(same in swift format lint).

@TTOzzi TTOzzi force-pushed the more-friendly-commands branch 4 times, most recently from d4c9aa0 to 6356aca Compare January 12, 2025 09:40
@allevato
Copy link
Member

This would remove the ability to format/lint from stdin without also passing --assume-filename, which could be a breaking change in some workflows.

Before breaking any of the current usage patterns, at a minimum I think we need to start allowing - to be specified as a path that represents stdin and give users time to migrate to that. If someone uses swift-format (lint) without any path arguments, we could emit a warning that says something to the effect of running swift-format with no input paths is deprecated and will be removed in the future; specify '-' to use stdin but otherwise keep the current behavior for a release.

Then, after that release, we should revisit all of the default behaviors discussed in that thread holistically, so that we don't risk ending up in an intermediate stage if we were to make them arbitrarily or piecemeal.

@TTOzzi TTOzzi force-pushed the more-friendly-commands branch from 6356aca to d73a539 Compare January 13, 2025 15:03
@TTOzzi
Copy link
Contributor Author

TTOzzi commented Jan 13, 2025

This would remove the ability to format/lint from stdin without also passing --assume-filename, which could be a breaking change in some workflows.

Before breaking any of the current usage patterns, at a minimum I think we need to start allowing - to be specified as a path that represents stdin and give users time to migrate to that. If someone uses swift-format (lint) without any path arguments, we could emit a warning that says something to the effect of running swift-format with no input paths is deprecated and will be removed in the future; specify '-' to use stdin but otherwise keep the current behavior for a release.

Then, after that release, we should revisit all of the default behaviors discussed in that thread holistically, so that we don't risk ending up in an intermediate stage if we were to make them arbitrarily or piecemeal.

Oh, I missed that it's possible to format/lint without the --assume-filename option. I modified the implementation so that it works correctly when code is provided via stdin.
Regarding this approach, I used isatty to check if there is input from the terminal for the swift-format command. Since I couldn't test it on Windows, I kept the existing behavior. However, I believe this improvement for Linux and macOS will be helpful.
I would appreciate your thoughts on this approach.
(I think this is also a temporary measure until we revisit the discussions regarding the default behaviors.)

@TTOzzi TTOzzi force-pushed the more-friendly-commands branch from d73a539 to 08f3f57 Compare January 13, 2025 15:12
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.

2 participants