search

senny / cabbage

get the maximum out of emacs
http://senny.github.com/cabbage/
156 stars 21 forks source link

Add feature "private bundles & private vendor dir" #153

Closed weiqiyiji closed 11 years ago

weiqiyiji commented 12 years ago

Hi, senny: Please take a look at the code, I'm looking forward to your suggestion!

senny commented 12 years ago

I think we need a function cabbage-vendor-library-path, which takes a library name as an argument and return the path to the library. This function should look through the cabbage-vendor-dirs in order to find the location where the library has been placed. You can then use this function internally and in the bundles to construct paths.

weiqiyiji commented 12 years ago

I keep cabbage-vendor-dir and cabbage-bundle-dir here, because some bundles have used the two variables. I just don't know if it needed to change them, that's why I ask for your suggestion.

Depends on your opinion, I will modify these code

Thank you! On Friday, September 14, 2012, Yves Senn wrote:

I think we need a function cabbage-vendor-library-path, which takes a library name as an argument and return the path to the library. This function should look through the cabbage-vendor-dirs in order to find the location where the library has been placed. You can then use this function internally and in the bundles to construct paths.

— Reply to this email directly or view it on GitHubhttps://github.com/senny/cabbage/pull/153#issuecomment-8537623.

senny commented 12 years ago

@weiqiyiji yes i think we should drop those variables and replace it with the function described above (at least for vendor-paths). In the case of bundle paths, please post the snippets of code you don't know how to replace.

weiqiyiji commented 12 years ago

(defun cabbage-python-init-snippets () (when (cabbage-bundle-active-p 'snippets) (add-to-list 'yas/root-directory (concat (concat cabbage-bundle-dir "python/snippets")) t) (yas/reload-all)))

The above code snippet is taken from bundles/python/bundle.el. The snippet load path depends on cabbage-bundle-dir, I don't have a good way to modify these codes.

On Fri, Sep 14, 2012 at 2:27 PM, Yves Senn notifications@github.com wrote:

@weiqiyiji https://github.com/weiqiyiji yes i think we should drop those variables and replace it with the function described above (at least for vendor-paths). In the case of bundle paths, please post the snippets of code you don't know how to replace.

— Reply to this email directly or view it on GitHubhttps://github.com/senny/cabbage/pull/153#issuecomment-8552778.

senny commented 12 years ago

I think this functionality needs to be rewritten. In my opinion the snippet bundle should traverse all active bundles and check if a snippet directory is present. If you it should add that directory to the yas/root-directory variable.

@jone what do you think?

jone commented 12 years ago

I agree. A general functionality for yas support in bundles would be aapreciated and would eliminate this code. The bundle loading function should handle this one.

weiqiyiji commented 11 years ago

Hi, senny, What should be cabbage-vendor-library-path return? The vendor library dir or library dot el file? Because some vendor libraries are placed in subdirs, but some others are directly put in vendor/ as a single dot el file

On Sat, Sep 15, 2012 at 2:23 AM, Jonas Baumann notifications@github.comwrote:

I agree. A general functionality for yas support in bundles would be aapreciated and would eliminate this code. The bundle loading function should handle this one.

— Reply to this email directly or view it on GitHubhttps://github.com/senny/cabbage/pull/153#issuecomment-8571033.

senny commented 11 years ago

@weiqiyiji I see. I think I need to experiment with the code a little so that we can ship this feature. Could you update the pull request with your latest changes? (you can to this by just pushing on your branch, github automatically reflects the changes in the PR). Please post a comment when you put your latest source online, I'll then have a look at the issues at hand.

weiqiyiji commented 11 years ago

@senny I haven't complete it yet. A little busy these days. I just wondering what's the return value of cabbage-vendor-library-path. I need to think more about this

senny commented 11 years ago

@weiqiyiji at the moment I'm not really sure. Thats why I would like you to push the latest code so that I can have a closer look at how things work out. This is a core functionality, which changes a lot of how the internals work and I can't see all the dependencies just thinking about it. It would be helpful to have the code to make some experiments and see what the best way is. I can't hint you in the right direction since at the moment I don't know which parts of cabbage need to change to support this feature in an ideal way.

weiqiyiji commented 11 years ago

@senny I've push my code online. My decision is to move every vendor library which is a single dot el file to a directory with the same name, I think this way can give users a consistent view. So, please review my code first, and any suggestion will be helpful.

weiqiyiji commented 11 years ago

@senny please try this commit on your emacs, I just added check for both buffer-file-name and load-file-name, and I'm not sure this one works in your config

weiqiyiji commented 11 years ago

Hi, @senny have you tried the newest commit?

senny commented 11 years ago

@weiqiyiji sorry, I did not get to it. I see that the branch also conflicts with the current master. Could you do a rebase?

@jone do you have a few minutes to give this a spin?

jone commented 11 years ago

hmm, something is wrong, I think. When I merge the branch some plone / python things do no longer work. e.g.

Somehow my initialization hooks are not executed. I have no errors.

I'm gonna look later into it when I have more time.

weiqiyiji commented 11 years ago

@senny @jone I think the problem is caused by (cabbage-load-bundle-dependencies), the way I get default-directory works differently in different machine. In my Mac, variable default-directory does not work, but

(file-name-directory (or buffer-file-name load-file-name))

works fine. So I'm confusing about how to deal this situation, and I can't figure out why this happen. Should I test both default-directory, buffer-file-name and load-file-name?

senny commented 11 years ago

@weiqiyiji I think we need a different strategy here. Using the bundle files relative path does not seem like a good option to tackle this problem.

@jone any ideas?

jone commented 11 years ago 
(file-name-directory (or buffer-file-name load-file-name))

works for me.

I think we should use the absolute cabbage paths as much as possible, although it does not work with in all situations (such as bundle snippets).

weiqiyiji commented 11 years ago

@jone if it doesn't work in all situations, I think it will be less useful. The problem now is how to use the absolute paths as much as possible, I don't find a good solution for this, any advice?

jone commented 11 years ago

@weiqiyiji I didn't really get the problem before, I think. So I played around a little. I've done some changes and pushed it to senny/private-bundle. You may pull from this branch or cherry-pick for getting it in this pull request, if you want..

Thank you for your work, it looks nice :+1: I'm looking forward to merging it ;)

Cheers

weiqiyiji commented 11 years ago

@jone thanks so much for your modification! I've tried your commit and it works great!

@senny do you have any opinions before you merge this PR?

senny commented 11 years ago

@weiqiyiji could you rebase before we review it? Currently Github tells me that the PR can't be merged.

weiqiyiji commented 11 years ago

@senny it's ok now, sorry for the conflicts...

jone commented 11 years ago

Works well for me :) @senny go ahead with merging if you want.

senny commented 11 years ago

HUGE PR and a big big thanks to @weiqiyiji for keep up the work. Thanks man!

Hope to see you around the cabbage project.

weiqiyiji commented 11 years ago

@jone @senny that's cool！