Skip to content

refactor: move option parsing into common.wrap()#479

Merged
ariporad merged 2 commits intomasterfrom
refactor-options-parsing
Jul 25, 2016
Merged

refactor: move option parsing into common.wrap()#479
ariporad merged 2 commits intomasterfrom
refactor-options-parsing

Conversation

@nfischer
Copy link
Copy Markdown
Member

This is roughly in-line with what was proposed for the plugin API. This moves option parsing as part of common.wrap(), so contributors don't need to worry about making the call themselves. By default, common.wrap() acts as it did before, and will only parse options if options are specified when the plugin is registered.

For trickier commands, like chmod() and set(), which sometimes need special parsing, the contributor can choose to parse these commands with an explicit call to common.parseOptions().

This deviates from the plugin API draft in that command options are passed in via the cmdOptions attribute, which I believe is more descriptive than an anonymous parameter to common.register(). This also makes it easier to leave option parsing optional.


### head([{'-n', \<num\>},] file [, file ...])
### head([{'-n', \<num\>},] file_array)
Available options:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why these changes? They seem unrelated.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ariporad As I was digging through, I saw that the documentation had an error (it was missing the available option). You're correct, these changes are unrelated to the refactor.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kk. LGTM

@ariporad
Copy link
Copy Markdown
Contributor

LGTM

@ariporad ariporad merged commit e438e61 into master Jul 25, 2016
@ariporad ariporad deleted the refactor-options-parsing branch July 25, 2016 00:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants