Add feature "private bundles & private vendor dir"

[weiqiyiji]

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

[senny]

Is this line necessary? I think we already require cl at the top of cabbage.el

[senny]

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]

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//pull/153#issuecomment-8537623.

[senny]

@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]

(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//pull/153#issuecomment-8552778.

[senny]

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]

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]

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//pull/153#issuecomment-8571033.

[senny]

@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]

@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]

@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]

@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.

[senny]

please move this function into lib/bundles/framework.el

[weiqiyiji]

@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]

Hi, @senny have you tried the newest commit?

[senny]

@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]

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

• in python I can no longer lookup imports (C-M-<return> in plone bundle)
• I can no longer run tests with M-e (configured in plone/buildout.el)

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

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

[weiqiyiji]

@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]

@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]

(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]

@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]

@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..

• The python problems were not related to the paths. The problem was that there was still a snippets loading hook in the plone bundle, which used a no longer existing path variable. If fixed it at 515baca
• The default-directory does not really work in the situation described above. It returns the directory path of the current buffer. But in this situation you need to have the path of the currently loading bundle file. I don't think this is possible.
  I've changed it so that cabbage-load-bundle-dependencies has an argument bundle, so that I can say from which bundle I'd like to load another lisp file (dependency).
  This has the advantage that it would be possible to override a complete dependency file by copying it into a local bundle directory with the same name (although I don't think that is something we want to do).
  See the changes in 9e77f09
• I've tested it locally and managed to move some local stuff into a local bundle and everything worked well (see https://github.com/jone/dotfiles/commit/0a9f27db8c185f08fd61eb4a1cdf4934588c0e34 ).
  I've also documented how I did it in 700d941, see https://github.com/senny/cabbage/tree/private-bundle#local-bundles

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

Cheers

[weiqiyiji]

@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]

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

[weiqiyiji]

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

[jone]

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

[senny]

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]

@jone @senny that's cool！