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 long options with object properties #924

Open
nfischer opened this issue Jan 4, 2019 · 3 comments
Open

Support long options with object properties #924

nfischer opened this issue Jan 4, 2019 · 3 comments
Labels
feature

Comments

@nfischer
Copy link
Member

nfischer commented Jan 4, 2019

Inspired by #923. It would be cool to support "long options" by using Object property names.

shell.touch({reference: 'ref-file.txt'}, 'foo.txt'); // same as '-r'
shell.rm({recursive: true}, 'foo.txt');
@nfischer nfischer mentioned this issue Jan 4, 2019
Closed
nfischer added a commit that referenced this issue Jan 4, 2019
@nfischer
feat(options): initial support for long options
ff7a8ae 
This adds initial support for long options. This integration tests them
with `shell.touch()` and unit tests them in `test/common.js`.

This also refactors some of the common internals:

 * Creates a new CommandError type to replace the `'earlyExit'` hack
 * Clearer Error types for `parseOptions()` exceptions
 * Removes side effects from a test which modifies `common.config`
 * Fixes appveyor and travis config to run lint (regressed in #920)

Issue #924
Test: touch.js, common.js
nfischer added a commit that referenced this issue Jan 4, 2019
@nfischer
feat(options): initial support for long options
a27a7c7 
This adds initial support for long options. This integration tests them
with `shell.touch()` and unit tests them in `test/common.js`. This
documents the new syntax.

This also refactors some of the common internals:

 * Creates a new CommandError type to replace the `'earlyExit'` hack
 * Clearer Error types for `parseOptions()` exceptions
 * Removes side effects from a test which modifies `common.config`
 * Fixes appveyor and travis config to run lint (regressed in #920)

Issue #924
Test: touch.js, common.js
@nfischer nfischer mentioned this issue Jan 4, 2019
Merged
nfischer added a commit that referenced this issue Jan 5, 2019
@nfischer
feat(options): initial support for long options (#926)
1560ffd 
* feat(options): initial support for long options

This adds initial support for long options. This integration tests them
with `shell.touch()` and unit tests them in `test/common.js`. This
documents the new syntax.

This also refactors some of the common internals:

 * Creates a new CommandError type to replace the `'earlyExit'` hack
 * Clearer Error types for `parseOptions()` exceptions
 * Removes side effects from a test which modifies `common.config`
 * Fixes appveyor and travis config to run lint (regressed in #920)

Issue #924
Test: touch.js, common.js

* small refactor for errorOptions
@Deadarius
Copy link

👍 to this one. How is the progress, can I help?

@nfischer
Copy link
Member Author

nfischer commented Jun 4, 2019

@Deadarius thanks for taking a look at this! Sorry for the delay, I've been quite busy lately.

I think #926 landed the initial framework. The next step is to go through existing commands and write tests to verify the existing long-options work (ex. rm({force: true}, ...)), and add those options to the README docs.

After that, we need to double-check we're using the right names for existing long-options (ex. cp('-n',...) should be {no_clobber: true}, cp('-f',...) should be {force: true})

@nfischer
Copy link
Member Author

nfischer commented Jun 4, 2019

#926 is a good starting point—look at the changes to src/touch.js and test/touch.js, you can skip over the rest.

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

No branches or pull requests
2 participants
@Deadarius @nfischer