Feedback on ELPA packaging

[purcell]

trafficstars

A few points of feedback having read through the code:

1. Perhaps it would even be possible to invoke probe.php using the command-line php executable and adjusting the PHP include_path to point at the project being inspected.
2. You should also include some ;;;###autoload cookies, at least for php-eldoc-enable, and I'd also suggest autoloading the setup hook:

;;;###autoload
(add-hook 'php-mode 'php-eldoc-enable)

1. ac-sources is not buffer-local, so you shouldn't add ac-source-php-eldoc to it every time php-eldoc-enable is called; either make ac-sources buffer-local, or just add the source once at global scope. (Making the var buffer local would be preferable IMO.)

Hope that helps!

-Steve

[sabof]

Thanks for the feedback.

1. Good idea, I'll have a look at it.

2. I've added a cookie for php-eldoc-enable, as you've suggested. I'm not a fan of self-installing packages. Someone might want to use the same functions in a different way.

3. You might be confusing ac-sources with another variable. This one has both default and buffer-local values. It's buffer-local by default.

Evgeni

[purcell]

2. I've added a cookie for php-eldoc-enable, as you've suggested. I'm not a fan of self-installing packages. Someone might want to use the same functions in a different way.

Fair enough!

3. You might be confusing ac-sources with another variable. This one has both default and buffer-local values. It's buffer-local by default.

Yes, you're right, sorry. I'm not sure if it was always buffer-local, but it certainly is now.

-Steve

[sabof]

I'll leave this bug open because of first point.