search

Slava / tern-meteor-sublime

Meteor Framework autocompletion for Sublime
246 stars 11 forks source link

Use package.js parent folder as root #7

Closed williamledoux closed 9 years ago

williamledoux commented 9 years ago

If a package.js file is found, take its parent as root so that the completion works also when editing a Meteor package.

Thanks you so much @Slava for this package, I had so much trouble to make ternJS work in ST3 before !

Slava commented 9 years ago

Thanks!

This is a good idea, but I need to test it myself, as you might have missed another point where the ternjs server is looking for the root: https://github.com/Slava/tern-meteor-sublime/blob/ee5e2e0960fbb9c606760b3593c3e93d561f3a9a/node_modules/tern/bin/tern#L38

I know it might be confusing as we are patching 2 places of original ternjs to have a custom logic.

williamledoux commented 9 years ago

I did see that you made changes in both places, but was not clear on why, so I changed the first one and it worked on my tests. Let me know if you want me to make the corresponding patch in ternjs server.

Oh, and while I have your attention, you have 4 people trying to give you money on flattr (by starring your repositories on github), so you may want to claim your account :) https://flattr.com/thing/3956519

Slava commented 9 years ago

@williamledoux interesting, I will look into this. I will be honest, my work in this repo was far hackier than anything I've done before, so a lot of the code-base I understand only vaguely.

I created an account on Flattr, but all those flattrs don't look like money :)

Slava commented 9 years ago

I thought more about this and the PR looks incorrect to me for another reason:

When you open the file in sublime, it starts a tern server if such is not running yet. If the first file you open is local package of some project, this code will think that the whole project is in package folder only, and not go up to the application. It is sort of correct if we treat packages as separate beasts but so far, I don't have the right code that separates packages vs apps.

williamledoux commented 9 years ago

I don't think it work the way you describe. Here is a screencast of a simple example with

  1. two unrelated apps (each one having a packages folder with a package inside)
  2. another unrelated package.
  3. sublime freshly started
  4. I added some logs to the console from tern.py to see what my patch was doing.

anim

What I see from this is that:

  1. completion is able to switch from one app to another just fine.
  2. I can access the app's collections from the package, which I find confusing. (that's because the tern server go back to the .meteor folder because it wasn't patched to stop at package.js)
  3. the completion on the third package is lazy, I can only see files that have been opened once. This would not happen if the server had been patched.

So I'm going to patch the server and see, tell me if I am missing something.

williamledoux commented 9 years ago

Seems to work as expected. I'm sorry I could not capture less than fullscreen.

anim

Slava commented 9 years ago

@williamledoux can you upload your example apps somewhere as one repo? I don't quite understand what you are demonstrating in gifs. I want to understand :)

(nice to see it working fine on elementaryos)

williamledoux commented 9 years ago

http://www.willdo.fr/share/github/tern-meteor-sublime_PR7.tar.gz In the last gif I check that

  1. When editing an app file I have only exports from this app and its packages (even if there is multiple apps in the project).
  2. When editing a package file, I have only the exports from the package (no matter if it is inside an app or not).
  3. On the right side are some logs of the patched tern.js file
Slava commented 9 years ago

Fair enough. After looking at the tarball with the example apps, I understand the gifs :)

Thank you for researching this, gonna merge.