feat(help): add help command #49

ariporad · 2016-05-05T23:41:48Z

No description provided.

ariporad · 2016-05-05T23:43:10Z

src/help.js

+import shell from 'shelljs';
+
+const commandList = Object.keys(shell)
+  .filter(cmd => typeof shell[cmd] === 'function' && cmd !== 'ShellString');


Ideally, this would be a part of shelljs itself.

Also, it would be nice if each command had a .help property which contained documentation. Maybe we could generate the documentation from that.

Or... (WARNING: Crazy Ahead):

Currently the docs are like this:

//@ Blah Blah Blah function _blah() { do_something(); }

What if we made them like this:

function _blah() { //@ Blah Blah Blah do_something(); }

AFAIK, it would work entirely the same, documentup doesn't actually care about the function, but then we could parse it like this:

var lines = shell[cmd].toString().split('\n').slice(1, -1); // Remove the first and last lines. var docLines = []; for (var i = 0; i < lines.length; ++i) { if (lines[i].strip().slice(0, 3) === '//@') { docLines.push(lines[i].strip().slice(3)); } else { break; } } shell[cmd].help = docLines.join('\n');

nfischer · 2016-05-05T23:46:45Z

@ariporad Can you add unit tests for this as well?

Also, the output looks off on my Linux box. I only see one character per line, which isn't ideal. Here's a screenshot of running shx help:

levithomason · 2016-05-05T23:47:15Z

I really think we should consider yargs here. It has solved all these issues and is well maintained. It allows defining commands, help, descriptions, examples, etc. It also a robust feature for specifying which args are required on which commands.

The output of help is also borked right now:
[redacted obnoxious output]

ariporad · 2016-05-05T23:50:25Z

@levithomason: The problem with yargs is that we have to more formally define the commands, vs. just calling shelljs.

Also, I fixed the output.

ariporad · 2016-05-05T23:50:54Z

src/help.js

+const commandList = Object.keys(shell)
+  .filter(cmd => typeof shell[cmd] === 'function' && cmd !== 'ShellString');
+
+const help = [


I'm totally open to help with the actual help message.

Can use a template string instead, they are multi-line:

const help = ` shx: A wrapper for shelljs UNIX commands. Usage: shx <command> [options] Example: $ shx ls . foo.txt bar.txt baz.js $ shx rm -rf *.txt $ shx ls . baz.js Commands: ${commandList.map(cmd => ` - ${cmd}`).join('\n')} `;

I added a Usage: part in there, feel free to rework this.

levithomason · 2016-05-05T23:52:32Z

I get the same output. Also, for starters, we could just loop through the shelljs commands and add the relevant parts to the yargs config. As shelljs exposed more info, we could beef that up.

nfischer · 2016-05-05T23:54:23Z

I don't know if we necessarily need yargs, since we're just trying to output a static string. Doesn't matter much to me though.

nfischer · 2016-05-05T23:55:18Z

Output definitely needs to be changed though, since it implies that shx cd is allowed (which it isn't).

levithomason · 2016-05-06T00:02:22Z

Output working, sorry had to rebuild!

ariporad · 2016-05-06T00:09:09Z

Should be fixed.

Thanks! Ari
On Thu, May 5, 2016 at 5:02 PM, Levi Thomason notifications@github.com wrote:
Output working, sorry had to rebuild!

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub [https://github.com//pull/49#issuecomment-217314054]

levithomason · 2016-05-06T00:11:19Z

src/help.js

@@ -0,0 +1,29 @@
+import shell from 'shelljs';
+
+const badCommands = ['ShellString', 'cd', 'pushd', 'popd', 'dirs'];


There is a CMD_BLACKLIST already defined in shx.js. Suggest pulling that and the ERROR_CODES into a config.js or something so it can imported into both shx.js and help.js.

Then you can filter based on the same list.

Done

Thanks! Ari
On Thu, May 5, 2016 at 5:11 PM, Levi Thomason notifications@github.com wrote:
In src/help.js [https://github.com//pull/49#discussion_r62274779] :

@@ -0,0 +1,29 @@
+import shell from 'shelljs';
+
+const badCommands = ['ShellString', 'cd', 'pushd', 'popd', 'dirs'];

There is a CMD_BLACKLIST already defined in shx.js . Suggest pulling that and the ERROR_CODES into a config.js or something so it can imported into both places.

Then you can filter based on the same list.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub [https://github.com//pull/49/files/c476547d608b00becee7cd1584e944c2d134c034#r62274779]

levithomason · 2016-05-06T00:16:34Z

Looking good, tested node 4/5/6. Just needs a trailing comma cleaned up in the commands list:

    - chmod
    - touch
    - tempdir
    - error,

ariporad · 2016-05-06T00:17:40Z

Done.x

levithomason · 2016-05-06T00:19:31Z

nit picky so feel free to ignore, but there are two line breaks at the end of the commands list. otherwise, LGTM.

    - tempdir
    - error


[system] in shx/ on add-help

levithomason · 2016-05-06T00:22:36Z

EDIT submitted before done

Would be nice to print the help after each error in shx as well:

  if (!fnName) {
    console.error('Error: Missing ShellJS command name');
    console.error(help());
    return EXIT_CODES.SHX_ERROR;
  }

  // validate command
  if (typeof shell[fnName] !== 'function') {
    console.error(`Error: Invalid ShellJS command: ${fnName}.`);
    console.error(help());
    return EXIT_CODES.SHX_ERROR;
  } else if (CMD_BLACKLIST.indexOf(fnName) > -1) {
    console.error(`Warning: shx ${fnName} is not supported`);
    console.error(help());
    return EXIT_CODES.SHX_ERROR;
  }

Then, we get helpful errors instead of just errors.

nfischer · 2016-05-06T00:53:12Z

I think it's most useful to have the full help only after shx and shx someNonexistentCommand. In the case of shx exec (an example blacklisted command) this could have a suggestion to run shx help in addition to the current message.

I like this because it keeps output succinct for simple mistakes (like shx cd), but shows how to get the full verbose info if that's necessary. That's just my thinking.

levithomason · 2016-05-06T00:54:46Z

@nfischer that is definitely superior. I'd sign off on that.

nfischer · 2016-05-10T21:27:42Z

@shelljs/contributors any further work on fixing the broken build/writing tests for this? I may be able to take up some work on this tonight or tomorrow.

levithomason · 2016-05-10T21:29:33Z

Been slammed here, will try to take a look tonight. No guarantees. If @ariporad is down, I say whoever can push it over the finish line should go for it.

ariporad · 2016-05-10T23:45:29Z

I'll try to work on this later tonight.

Also shows help on `shx nonExistantCommand`

ariporad · 2016-05-11T15:01:12Z

Ok, tests added, and it should print help on shx, shx help, and shx nonexistantcommand. Prints a message to use help on shx exec (or any other blacklisted command).

levithomason · 2016-05-11T15:04:00Z

Per @nfischer's comment, i think it would be best to not show the full help when missing a command or an invalid command is used, but instead, just show the error plus something like "For more info: shx help"

Other than that, this looks great!

codecov-io · 2016-05-11T15:29:44Z

Current coverage is 90.32%

Merging #49 into master will increase coverage by +6.11%

@@             master        #49   diff @@
==========================================
  Files             2          4     +2   
  Lines            19         31    +12   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits             16         28    +12   
  Misses            3          3          
  Partials          0          0

Powered by Codecov. Last updated by be59e0f...9321db6

levithomason · 2016-05-11T18:11:45Z

Pulled and tested, LGTM. Going to merge this.

nfischer · 2016-05-11T20:57:05Z

Awesome, thanks for updating the tests 👍

feat(help): add help command

85af899

ariporad reviewed May 5, 2016
View reviewed changes

fix(help): we can have >1 chars/line

87a5105

ariporad added docs feat high priority labels May 5, 2016

ariporad reviewed May 5, 2016
View reviewed changes

ariporad added 2 commits May 5, 2016 17:05

fix(help): shx cd is a no-no

1346435

refactor(help): use template strings

c476547

levithomason reviewed May 6, 2016
View reviewed changes

refactor: Move stuff into config.js

0c4f198

fix: remove comma

860d3e1

tests(help); add tests for help

0d745c1

Also shows help on `shx nonExistantCommand`

levithomason merged commit b893002 into master May 11, 2016

levithomason deleted the add-help branch May 11, 2016 18:12

		@@ -0,0 +1,29 @@
		import shell from 'shelljs';

		const badCommands = ['ShellString', 'cd', 'pushd', 'popd', 'dirs'];

feat(help): add help command #49

feat(help): add help command #49

Uh oh!

Conversation

ariporad commented May 5, 2016

Uh oh!

ariporad May 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfischer commented May 5, 2016

Uh oh!

levithomason commented May 5, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ariporad commented May 5, 2016

Uh oh!

ariporad May 5, 2016

Choose a reason for hiding this comment

Uh oh!

levithomason May 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

levithomason commented May 5, 2016

Uh oh!

nfischer commented May 5, 2016

Uh oh!

nfischer commented May 5, 2016

Uh oh!

levithomason commented May 6, 2016

Uh oh!

ariporad commented May 6, 2016

Uh oh!

levithomason May 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ariporad May 6, 2016

Choose a reason for hiding this comment

Uh oh!

levithomason commented May 6, 2016

Uh oh!

ariporad commented May 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

levithomason commented May 6, 2016

Uh oh!

levithomason commented May 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nfischer commented May 6, 2016

Uh oh!

levithomason commented May 6, 2016

Uh oh!

nfischer commented May 10, 2016

Uh oh!

levithomason commented May 10, 2016

Uh oh!

ariporad commented May 10, 2016

Uh oh!

ariporad commented May 11, 2016

Uh oh!

levithomason commented May 11, 2016

Uh oh!

codecov-io commented May 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Current coverage is 90.32%

Uh oh!

levithomason commented May 11, 2016

Uh oh!

nfischer commented May 11, 2016

Uh oh!

Uh oh!

ariporad May 5, 2016 •

edited

Loading

levithomason commented May 5, 2016 •

edited

Loading

levithomason May 6, 2016 •

edited

Loading

levithomason May 6, 2016 •

edited

Loading

ariporad commented May 6, 2016 •

edited

Loading

levithomason commented May 6, 2016 •

edited

Loading

codecov-io commented May 11, 2016 •

edited

Loading