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.

Sign up for GitHub

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

Jump to bottom

Support standard input via shell redirection with zk new #240

Merged
merged 1 commit into from
Jul 10, 2022
Merged

Support standard input via shell redirection with zk new #240

merged 1 commit into from
Jul 10, 2022

Conversation

mickael-menu
Copy link
Member

Fixed

  • #239 Support standard input via shell redirection with zk new.

@mickael-menu mickael-menu merged commit 53aabce into main Jul 10, 2022
@mickael-menu mickael-menu deleted the issue-239 branch July 10, 2022 17:10
@pkazmier
Copy link
Contributor

This change broke interactive use of zk new as it now sits indefinitely waiting for input on standard input unless you press Ctrl-D. I think the best approach here is to offer a command line argument -i or --interactive for the new command. If set, then attempt to read from standard input. Thoughts?

pkazmier added a commit to pkazmier/zk that referenced this pull request Jul 11, 2022
@pkazmier
Add explicit flag to read from standard input
c4ed7c0 
Fixes bug introduced in zk-org#240 where interactive use of `zk new` results
in the command sitting indefinitely for user to supply user input.

This change introduces an explicit flag to `new` to indicate standard
input should be read for the content of the note.  The flag is called
`-i` on `--input`.

The flag name is somewhat confusing as there is a `--no-input` flag at
the global level.  I propose that we change that flag name to
`--no-prompt` as it more accurately describes the option.  To maintain
backwards compatibility, we define `--no-input` as an alias.
@pkazmier pkazmier mentioned this pull request Jul 11, 2022
Merged
@pkazmier
Copy link
Contributor

I submitted #242 with this suggestion. I updated the tests, docs, and made small code change as you'll see. I also renamed --no-input to --no-prompt as the flag I added was -I / --input, but I did add make --no-prompt backwards compatible by adding an alias for --no-input I believe. I know this is your baby, so I won't be offended if you don't care for any of the suggestions. I defer to you.

@mickael-menu
Copy link
Member Author

Ha crap, the tesh files didn't pick up on this. I'll answer directly on your PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mickael-menu @pkazmier