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

Add a no-timer option to not print timer during countdown #9

Merged
merged 4 commits into from
Oct 7, 2016

Conversation

mikaylathompson
Copy link
Contributor

No description provided.

@mikaylathompson mikaylathompson mentioned this pull request Oct 7, 2016
@liviu-
Copy link
Owner

liviu- commented Oct 7, 2016

Great, thanks a lot, I'll go ahead and merge it.

It's a bit unfortunate that it only supports the optional argument at the end, cause people can't $ alias ding-bg="ding --no-timer", but in light of the discussion in #8, I think I should add some sorts of parse_optional_args() function when I implement the issue there anyway.

@liviu- liviu- merged commit 5a05c87 into liviu-:develop Oct 7, 2016
@mikaylathompson
Copy link
Contributor Author

Agreed on more flexibility in the ordering.

I think that at some point it'll make sense to add something like argparse (I'm actually quite partial to click, but that would introduce a dependency) to deal with more complex options.

For what it's worth, my bashrc contains:
function ding() { /path/to/ding.py $@ --no-timer&}

so I can still call it as ding in 3s

@liviu-
Copy link
Owner

liviu- commented Oct 7, 2016

Yeah, I'll probably end up using argparse, though I don't think it allows shared arguments among subparsers both at the beginning and at the end of the command unfortunately.

If it's a choice between $ ding --no-timer in 1s and $ ding in 1s --no-timer, which one do you think would be better? I see the former one as more friendly to alias, but the latter one a bit more natural as it feels like one would first think about the time, and only after about the options they may prefer.

I will do this tomorrow among with adding support for #8.

liviu- added a commit that referenced this pull request Oct 8, 2016
The argument parser is now implemented with `argparse`, but the
optional flag may only be provided at the end of the command.
@liviu- liviu- mentioned this pull request Oct 8, 2016
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