fix for the handling of bad input in "mojito jsling mojit" command #542

Merged
merged 2 commits into from Dec 10, 2012

Projects

None yet

4 participants

@dmitris
Contributor
dmitris commented Sep 21, 2012

Currently running the command:
mojito jslint mojit
produces a not so user-friendly Javascript error:

$ mojito jslint mojit
path.existsSync is now called fs.existsSync.

/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:312
var parts = mojitPath.split('/');
^
TypeError: Cannot call method 'split' of undefined
at getMojitName (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:312:27)
at Object.run (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:451:13)
at main (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/management/cli.js:125:13)
at Object. (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/management/cli.js:137:1)
at Module._compile (module.js:449:26)
at Object.Module._extensions..js (module.js:467:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:312:12)
at Module.require (module.js:362:17)
at require (module.js:378:17)

the same if you mistype the path to the mojit:

I fixed the handling of the missing and incorrect input and modified the usage/help message to be:

mojito jslint [app | mojit] [] {options}
- target: app or mojit
- path (required for a mojit target): the path to the mojit to run jslint on

Run jslint on the app in the current directory:
mojito jslint app .
Run jslint on mojits/Bar:
mojito jslint mojit ./mojits/Bar
Run jslint on the mojito framework itself!
mojito jslint

OPTIONS:
--print : print results to stdout
-p : short for --print

(which would also highlight the fact that "mojito jslint" runs checks on the Mojito framework itself, not the app in the current directory).

jslint output on lib/app/commands/jslint.js is clean. The tests seem to run fine ("mojito test").

I was not sure - am I supposed to write unit or other tests for the command-line handlers? Please let me know, if required I will add them. Thanks.

@dmitris
Contributor
dmitris commented Sep 21, 2012

Here's the current error if you mistype the mojito name (path):

$ mojito jslint mojit mojits/QueryMojito

fs.js:500
return binding.readdir(pathModule._makeLong(path));
^
Error: ENOENT, no such file or directory '/Users/dmitris/dev/hack/my-mojito/examples/developer-guide/using_parameters/mojits/QueryMojito'
at Object.fs.readdirSync (fs.js:500:18)
at processDir (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:93:20)
at processFiles (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:361:5)
at Object.run (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/app/commands/jslint.js:478:14)
at main (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/management/cli.js:125:13)
at Object. (/Users/dmitris/.nvm/v0.8.9/lib/node_modules/mojito/lib/management/cli.js:137:1)
at Module._compile (module.js:449:26)
at Object.Module._extensions..js (module.js:467:10)
at Module.load (module.js:356:32)
at Function.Module._load (module.js:312:12)

I changed it to:

$ mojito jslint mojit mojits/QueryMojito
✖ Mojit mojits/QueryMojito not found.
usage:
mojito jslint [app | mojit] [] {options}
- target: app or mojit
- path (required for a mojit target): the path to the mojit to run jslint on

Run jslint on the app in the current directory:
mojito jslint app .
Run jslint on mojits/Bar:
mojito jslint mojit ./mojits/Bar
Run jslint on the mojito framework itself!
mojito jslint

OPTIONS:
--print : print results to stdout
-p : short for --print

@isao
Contributor
isao commented Sep 21, 2012

+1

@FabianFrank
Contributor

+1

@lzhan lzhan was assigned Dec 6, 2012
@lzhan lzhan merged commit 740af31 into yahoo:develop Dec 10, 2012

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment