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.

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

[CS2] Un-prefer global #4543

Merged
merged 2 commits into from
May 9, 2017
Merged

Conversation

GeoffreyBooth
Copy link
Collaborator

@GeoffreyBooth GeoffreyBooth commented May 8, 2017

Closes #4049.

  • Installing CoffeeScript no longer triggers NPM’s “prefer global” warning
  • The coffee and cake commands now first check if you have a locally-installed CoffeeScript module under the current path, and execute that version if so, similar to how Gulp works; and fall back to the global version otherwise.

This enables using different versions of the CoffeeScript CLI on a per-project basis:

/tmp ▶ coffee -v
CoffeeScript version 1.12.5
/tmp ▶ mkdir coffeescript1.11
/tmp ▶ cd coffeescript1.11
/tmp/coffeescript1.11 ▶ npm install coffeescript@1.11.0
/private/tmp/coffeescript1.11
└── coffeescript@1.11.0

npm WARN enoent ENOENT: no such file or directory, open '/private/tmp/coffeescript1.11/package.json'
npm WARN coffeescript1.11 No description
npm WARN coffeescript1.11 No repository field.
npm WARN coffeescript1.11 No README data
npm WARN coffeescript1.11 No license field.
/tmp/coffeescript1.11 ▶ coffee -v
CoffeeScript version 1.11.0
/tmp/coffeescript1.11 ▶ cd ..
/tmp ▶ mkdir coffee-script1.11
/tmp ▶ cd coffee-script1.11
/tmp/coffee-script1.11 ▶ npm install coffee-script@1.11.0
/private/tmp/coffee-script1.11
└── coffee-script@1.11.0

npm WARN enoent ENOENT: no such file or directory, open '/private/tmp/coffee-script1.11/package.json'
npm WARN coffee-script1.11 No description
npm WARN coffee-script1.11 No repository field.
npm WARN coffee-script1.11 No README data
npm WARN coffee-script1.11 No license field.
/tmp/coffee-script1.11 ▶ coffee -v
CoffeeScript version 1.11.0
/tmp/coffee-script1.11 ▶ cd ..
/tmp ▶ mkdir coffeescript2
/tmp ▶ cd coffeescript2
/tmp/coffeescript2 ▶ npm install coffeescript@next
/private/tmp/coffeescript2
└─┬ coffeescript@2.0.0-beta1
  └─┬ markdown-it@8.3.1
    ├─┬ argparse@1.0.9
    │ └── sprintf-js@1.0.3
    ├── entities@1.1.1
    ├── linkify-it@2.0.3
    ├── mdurl@1.0.1
    └── uc.micro@1.0.3

npm WARN enoent ENOENT: no such file or directory, open '/private/tmp/coffeescript2/package.json'
npm WARN coffeescript2 No description
npm WARN coffeescript2 No repository field.
npm WARN coffeescript2 No README data
npm WARN coffeescript2 No license field.
/tmp/coffeescript2 ▶ coffee -v
CoffeeScript version 2.0.0-beta1

If you don’t have a globally-installed version, the coffee or cake commands don’t do anything; you either have to use the locally-installed module via Node, like a normal module, or access the CLI via ./node_modules/coffeescript/bin/coffee.

…uld try to run the locally-installed module if it exists, or the global version otherwise
@GeoffreyBooth GeoffreyBooth added this to the 2.0.0 milestone May 8, 2017
@GeoffreyBooth GeoffreyBooth requested a review from lydell May 8, 2017 00:45
bin/cake Outdated
path.join(path.dirname(fs.realpathSync(__filename)), '../lib/coffeescript')
];

for (var i = 0; i < 4; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a hard coded 4 rather than potentialPaths.length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was trying to be optimized, though I can see how this comes across as overkill. I toyed with a series of if statements to avoid even the unnecessary path manipulation lines, but testing didn't show a difference. I can make this more idiomatic if you think it's silly to hard-code the length to save 1ms.

bin/cake Outdated
path.join(process.cwd(), 'node_modules/coffeescript/lib/coffeescript'),
path.join(process.cwd(), 'node_modules/coffeescript/lib/coffee-script'),
path.join(process.cwd(), 'node_modules/coffee-script/lib/coffee-script'),
path.join(path.dirname(fs.realpathSync(__filename)), '../lib/coffeescript')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it was like this before, but why not use __dirname?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dunno. Was there a reason it was how it was before? Windows?

@GeoffreyBooth
Copy link
Collaborator Author

@lydell how about now?

@GeoffreyBooth GeoffreyBooth merged commit 993347b into jashkenas:2 May 9, 2017
GeoffreyBooth added a commit to GeoffreyBooth/coffeescript that referenced this pull request May 9, 2017
@GeoffreyBooth GeoffreyBooth deleted the un-prefer-global branch May 12, 2017 05:38
GeoffreyBooth added a commit that referenced this pull request May 14, 2017
* Un-prefer global (#4543)

* 1.12.6 changelog; update NPM module in documentation to be `coffeescript` instead of `coffee-script`; update installation to add note about global vs local `coffee` command

* Update packages

* Updated output

* Simplify changelog
mikermcneil added a commit to mikermcneil/grunt that referenced this pull request Jul 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants