refactor: expose plugin utils & add initial tests#484
Merged
Conversation
| // http://github.com/arturadib/shelljs | ||
| // | ||
|
|
||
| var commonPath = './src/common'; |
Contributor
There was a problem hiding this comment.
Why are we deleting this?
Member
Author
Member
Author
|
@ariporad I just fixed the issue with node v0.10-12. This should be good now. |
Member
Author
|
For reference, the bug I found with the approach in #482 is that there are two different instances of A plugin might signal an error using |
Contributor
|
LGTM! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After more experimentation, I realized that #482 wasn't a perfect approach, but that this would be a better one. Even after #482,
shell.error()was actually broken. This fixes that issue.This also starts differentiating internal implementations (
src/common.js) from exposed parts of the plugin API (plugin.js). This is of course still far from a complete plugin API, but this should be enough to make simple plugins (and still get benefits like globbing, shelljs-style error signaling, etc.).All implementations are still within
src/common.js, and internal commands still rely on that directly, but external plugins are intended to usevar plugin = require('shelljs/plugin')to get functions likeplugin.register(). This also makes it easy to modify the plugin API without breaking ShellJS commands.I also started unit tests for the plugin API. This creates one sample plugin (with all the bells and whistles like automatic option parsing) and then makes sure it works properly. We may also want to create a simpler plugin (i.e. no automatic option-parsing, no wrapping the return value, etc.) and test that.
If you have any suggestions on naming conventions, or think there's a better approach to this issue, let me know. Like I said, this is still far from complete, but suggestions are welcome.