Skip to content

install phpactor with phpactor-update function #60

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

Merged
merged 1 commit into from
Oct 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions README.org
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,20 @@ This package is Emacs interface to [[http://phpactor.github.io/phpactor/][Phpact

*NOTICE*: This package is in development. Since some functions are running, this is released as an /alpha version/.

*NOTICE*: Phpactor is also in development stage.

#+BEGIN_HTML
<a href="http://melpa.org/#/phpactor"><img alt="MELPA: phpactor" src="http://melpa.org/packages/phpactor-badge.svg"></a>
<a href="http://stable.melpa.org/#/phpactor"><img alt="MELPA stable: phpactor" src="http://stable.melpa.org/packages/phpactor-badge.svg"></a>
#+END_HTML
** Instalation
Please be aware that Phpactor is also in development stage.
A simple installation method is not yet provided, but you can build it using Composer.

After having installed this package, run `phpactor-update` (this will install a supported version of phpactor inside phpactor.el's installation folder).
*NOTICE*: To ensure the appropriate version of Phpactor is installed, please run this command after every upgrade of phpactor.el

Alternatively, you can install Phpactor on your own and configure `phpactor-executable` but please be aware that any change in Phpactor's rpc protocol can introduce breakages.


** Configuration
See https://phpactor.github.io/phpactor/configuration.html

Expand Down
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
"sebastian/diff": "@dev"
},
"config": {
"sort-packages": true
"optimize-autoloader": true,
"classmap-authoritative": true,
"sort-packages": true
}
}
26 changes: 21 additions & 5 deletions phpactor.el
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
;; Created: 8 Apr 2018
;; Version: 0.1.0
;; Keywords: tools, php
;; Package-Requires: ((emacs "24.3") (cl-lib "0.5") (f "0.17"))
;; Package-Requires: ((emacs "24.3") (cl-lib "0.5") (f "0.17") (composer "0.1"))
;; URL: https://github.com/emacs-php/phpactor.el
;; License: GPL-3.0-or-later

Expand Down Expand Up @@ -48,6 +48,7 @@
(require 'json)
(require 'php-project)
(require 'ring)
(require 'composer)

;; Variables
;;;###autoload
Expand Down Expand Up @@ -86,10 +87,23 @@
"Return Phpactor command or path to executable."
(or (when phpactor-executable
(php-project--eval-bootstrap-scripts phpactor-executable))
(executable-find phpactor-command-name)
(let ((vendor-executable (f-join (phpactor-get-working-dir) "vendor/bin/phpactor")))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was a bit puzzled by this way of installing phactor (inside a php project), but even so, I'd rather recommend everyone to either install it from phpactor.el or define it manually (hopefully after having considered the risks it brings).

how does that sound ?

(let ((vendor-executable (f-join (phpactor-package-directory) "vendor/bin/phpactor")))
(when (file-exists-p vendor-executable)
vendor-executable))))
vendor-executable))
(error "Phpactor not found. Please run phpactor-update")))

(defun phpactor-update ()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about naming phpactor-setup instead of phpactor-update?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I tend to like the semantic that comes with the term update as it conveys the idea that sooner or later, you'll have to run it again to stay up to date.

But I agree that for the first time you use it, it feels a bit wrong. Would that justify having both name, one aliasing towards the other ?

"Install or update phpactor inside phpactor.el's folder."
(interactive)
(let ((package-folder (phpactor-package-directory))
(composer-executable (car (composer--find-executable))))
(unless composer-executable (error ("composer not found.")))
(setq default-directory package-folder)
(call-process composer-executable nil (get-buffer-create phpactor-action--buffer-name) nil "install" "--no-dev")))

(defun phpactor-package-directory ()
"Return the folder where phpactor.el is installed."
(file-name-directory(locate-library "phpactor.el")))

(defun phpactor-get-working-dir ()
"Return working directory of Phpactor."
Expand Down Expand Up @@ -464,7 +478,9 @@ function."
(cl-defun phpactor-action-dispatch (&key action parameters version)
"Execite action by `NAME' and `PARAMETERS'."
(when (and version (not (equal phpactor--supported-rpc-version version)))
(error "Phpactor uses rpc protocol %s whereas this package requires %s" version phpactor--supported-rpc-version))
(if (phpactor-executable)
(error "Phpactor uses rpc protocol %s but this package requires %s" version phpactor--supported-rpc-version)
(error "Phpactor should be upgraded. Please run phpactor-update")))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These lines can be modified as follows, but what do you think?

@@ -469,10 +469,11 @@ function."
 ;; Dispatcher:
 (cl-defun phpactor-action-dispatch (&key action parameters version)
   "Execite action by `NAME' and `PARAMETERS'."
-  (when (and version (not (equal phpactor--supported-rpc-version version)))
-    (if (phpactor-executable)
+  (unless (or (null version)
+              (version<= phpactor--supported-rpc-version version))
+    (if (phpactor-find-executable)
         (error "Phpactor uses rpc protocol %s but this package requires %s" version phpactor--supported-rpc-version)
-        (error "Phpactor should be upgraded. Please run phpactor-update")))
+      (error "Phpactor should be upgraded.  Please run phpactor-update")))
   (phpactor--add-history 'phpactor-action-dispatch (list :action action :parameters parameters :version version))
   (let ((func (cdr-safe (assq (intern action) phpactor-action-table))))
     (if func

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, I'm not yet used to unless so it takes me a little bit more effort to read...

version<= phpactor--supported-rpc-version version

Not sure I see what you mean here. if versions are equal, no issue but if version is different than phpactor--supported-rpc-version version, I think we should stop with an error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I see.

(phpactor--add-history 'phpactor-action-dispatch (list :action action :parameters parameters :version version))
(let ((func (cdr-safe (assq (intern action) phpactor-action-table))))
(if func
Expand Down