-
Notifications
You must be signed in to change notification settings - Fork 10
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
🐛 fix --stdin
CLI option
#57
Conversation
Thank you @amtoine ! With your help I could solve the issue 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes look ok to me, but i'm not sure they address the original issue 🤔
i'm running the following, on top of the PR branch and after a cargo clean
:
cargo build --release
to create./target/release/nufmt
foo out> foo.txt
to create a dummy file
and then
./target/release/nufmt --stdin (open foo.txt)
still works as beforeopen foo.txt | ./target/release/nufmt
gives an erroropen foo.txt | ./target/release/nufmt --stdin
also gives an error
i expected
1.
to not work anymore2.
to give an error as it did3.
to work as 1. does
Hello! I break the API and forgot to tell you 😆 . Thank you for listing every case. |
superseded by #61 |
## Description of changes `--stdin` flag read a command line argument, not stdin. Now it reads from stdin. Not sure if my code would compile on windows, but I don't have a windows computer to test it. I'm aware that #57 tries to solve this as well, but it seems stagnant and won't pass CI. ## Relevant Issues #56
Description of changes
WIP
Relevant Issues
#56