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

meteor plugin as installed package. See http://ternjs.net/doc/manual.html#plugin_third_party #17

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

angelozerr
Copy link
Contributor

This PR gives you the capability to avoid setting meteor.js inside tern/plugin folder but setting tern-meteor folder inside your node_modules. More once you will publish tern-meteor with npm publish, you will able to install it with npm install. See http://ternjs.net/doc/manual.html#plugin_third_party for explanation how tern loads tern plugin.

@angelozerr
Copy link
Contributor Author

@Slava if you wish some more explanation about my (very simple) patch, don't hesitate to tell me. I need this patch to integrate tern-meteor inside tern.java (see https://github.com/angelozerr/tern.java/wiki/Tern-&-Meteor-support) with clean mean (not inside tern/plugin but inside node_modules/tern-meteor folder)

Thank's!

@Slava
Copy link
Owner

Slava commented Nov 1, 2014

I didn't see it, sorry.

Some explanation on how it is supposed to work and updated installation docs (they change if we publish this as npm module, right?) would be great.

@angelozerr
Copy link
Contributor Author

At first, with my patch you can publish tern-meteor with npm publish.

Today you must copy/paste at hand the meteor.js inside the tern/plugin folder, with my patch you can use npm install. Here steps :

  • create an empty TernJS folder.
  • cd TernJS
  • npm install tern to download tern. After that your TernJS folder will contains node_modules/tern
  • cd TernJS/node_modules/tern
  • npm install tern-meteor to download tern meteor. After that your TernJS/node_modules/tern folder will have a new folder node_modules/tern-meteor

And that's all!

To use npm install tern-meteor you will have to publish tern-meteor (with npm punblish).

My patch should not break the existing installation (you will able to copy/paste meteor.js inside tern/plugin if you wish).

@Slava
Copy link
Owner

Slava commented Nov 2, 2014

I suggest you do it yourself and then give me access to the npm module you publish so I can update in future in case you are not around. I am not ready to merge things I don't understand and I don't have time to read up about this so far.

@angelozerr
Copy link
Contributor Author

I am not ready to merge things I don't understand and I don't have time to read up about this so far.

that's shame-(

Other tern plugin does that. For instance foogle closure provides that. You can read installation guide of tern-closure at https://github.com/google/tern-closure#short-version

@angelozerr
Copy link
Contributor Author

@Slava if you decide not to accept this PR, could you just accept please only this update https://github.com/Slava/tern-meteor/pull/17/files#diff-0e3c470f571980d185893bb3b30a4164R3

This update works when meteor.js is inside tern/plugin (like today) and will work when tern plugin is outside tern/plugin (inside tern-meteor foldr with a package.json).

Thank's!

@Ulexus
Copy link

Ulexus commented Sep 29, 2015

+1

This PR is trivial enough. If @Slava isn't inclined, though, I would agree that you ( @angelozerr ) should just publish tern-meteor to NPM yourself. Just fork and publish.

Worst case, if you don't want to do it, I'll be happy to do so.

@angelozerr
Copy link
Contributor Author

IMHO, I think it should be better that @Slava publish it, because he knows when tern-meteor can be released.

@Ulexus
Copy link

Ulexus commented Sep 30, 2015

I would not disagree; on the other hand, it's been a year.

ferhtgoldaraz pushed a commit to ferhtgoldaraz/tern-meteor that referenced this pull request Nov 7, 2015
Followed a PR that worked on npm publication:
Slava#17
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