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

Non shitty parser errors / no per-task argspec help #57

Merged
merged 42 commits into from Jul 17, 2013

Conversation

bitprophet
Copy link
Member

  • When you don't give enough positional args it doesn't say which ones weren't given
  • When you give an unknown flag it doesn't say what is available
    • AKA there's no per-task help, this is a must-have

@bitprophet
Copy link
Member Author

This requires the dreaded "optional argument value" parser implementation: when a flag can optionally take a value, what does one do with the next token if there is any ambiguity (versus unfilled posargs or values that look like valid task names or other flags).

I think the best way for now is to refuse the temptation to guess, as per PEP20, and raise an error whenever ambiguity strikes. Will take a stab at this and see how hard that is to do.

@bitprophet
Copy link
Member Author

Picking this back up...

  • Have it setting non-given values to True, parser-wise (e.g. --foo => sets foo = True, instead of to foo's default value).
  • However, our factory shit is fucking this up: most optional-value flags will want to take a string if they're given a real value. BUT when they are given "as" a boolean, we want to give them a boolean value.
    • raw_value is being set correctly, but this doesn't help us much here, we want both to end up 'right' here; instead the regular value ends up as 'True' which is silly :)
  • Maybe double check how argparse & friends handle this?
  • Otherwise, obvious sol'ns:
    • Have Argument's value setter explicitly test for this (e.g. if self.option is true + value is a bool: don't pass thru self.kind)
    • Add a new "no I really want you to set this without casting it" API call on Argument, like set_value or raw_set or some shit
    • Default optional args to having a custom kind factory class that allows either bools or strings
  • Leaning towards those in that order, I guess.

@bitprophet
Copy link
Member Author

Actually I think the "explicit API call" is cleanest, let's do that.

@bitprophet
Copy link
Member Author

Womp, those 2 failing tests now pass \o/ Time to fill in the other 3 skipped ones, then make sure there's a good high level test (or that it's used to fill in per-task help shit).

@bitprophet
Copy link
Member Author

Skipped tests: filled. Hopefully in bidness.

Wrote failing test for e.g. inv -h taskname - still gotta think a bit more on how that should really be structured (e.g. display docstring? add note about 'to see core options give -h with no task name'? etc) too.

@bitprophet
Copy link
Member Author

Split off #76 for detailed help shit, going with a basic-ish view (aping core help, of course) for now.

@bitprophet
Copy link
Member Author

Per-task help output works in basic form now!

What's missing still:

  • Visually specify brackets around the =STRING crap in per-arg help, for optional values (i.e. core --help needs to imply that giving a string is optional)
  • Not sure I ever actually set up an API at the task-arg level for specifying optional-ness...it's not in the docs anyway. Need this so I can actually set up a test task!

@bitprophet
Copy link
Member Author

Done, and done. And...done! unless I broke python 3 somewhere, will find out soon.

@bitprophet
Copy link
Member Author

I did break it! (Initially in master. Heh.) Fixed now locally, waiting for Travis to confirm.

@bitprophet
Copy link
Member Author

GREEN

bitprophet added a commit that referenced this pull request Jul 17, 2013
Non shitty parser errors / no per-task argspec help
@bitprophet bitprophet merged commit 639e7a6 into master Jul 17, 2013
@bitprophet bitprophet deleted the per-task-help-53 branch July 17, 2013 03:39
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.

None yet

1 participant