Skip to content

Add trymodule - #477

Closed
victorb wants to merge 1 commit into
sindresorhus:masterfrom
victorb:patch-1
Closed

Add trymodule#477
victorb wants to merge 1 commit into
sindresorhus:masterfrom
victorb:patch-1

Conversation

@victorb

@victorb victorb commented Mar 20, 2016

Copy link
Copy Markdown
Contributor

Made a simple CLI for trying out different node modules directly from the terminal. Helps developers to evaluate packages without having to add to an already existing project so hopefully this is helpful for many people.

@sindresorhus sindresorhus changed the title Add trymodule to command line apps Add trymodule Mar 29, 2016
sindresorhus pushed a commit that referenced this pull request Mar 29, 2016
@sindresorhus

Copy link
Copy Markdown
Owner

https://github.com/VictorBjelkholm/trymodule/blob/2030806d790213d426456959fdc7a69773a4c6f7/index.js#L9

Use os.homedir().

https://github.com/VictorBjelkholm/trymodule/blob/2030806d790213d426456959fdc7a69773a4c6f7/index.js#L59

Oh noes. Please don't. Never do string concat for commands, use execFile instead. Never pass environment variables directly to rm without sanitation, better yet, never spawn rm. Use a JS tools, like rimraf or even better del.

https://github.com/VictorBjelkholm/trymodule/blob/2030806d790213d426456959fdc7a69773a4c6f7/index.js#L5

You might want to reconsider using colors. It's a bad unmaintained module from a user-hostile author. Check out cli-colors or chalk instead.

@victorb

victorb commented Mar 29, 2016

Copy link
Copy Markdown
Contributor Author

@sindresorhus thanks for merging but even more thanks for the feedback! It's awesome!

I'll look into solving these issues. Thanks again for taking the time!

@sbruchmann

Copy link
Copy Markdown

@sindresorhus wrote:

You might want to reconsider using colors. It's a bad unmaintained module from a user-hostile author.

What’s the problem with Marak?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants