Skip to content
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

wip : xref backend experiment #128

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

wip : xref backend experiment #128

wants to merge 2 commits into from

Conversation

kermorgant
Copy link
Contributor

@kermorgant kermorgant commented Aug 7, 2019

I wanted to see how it would behave.

attention, this is based on branch feature/phpactor-executable-defcustom-take2

@kermorgant kermorgant changed the base branch from master to feature/phpactor-executable-defcustom-take2 August 11, 2019 14:47
@kermorgant
Copy link
Contributor Author

can be tested with such function

(defun mk/phpactor-xref ()
  "Activates xref backend using phpactor."
  (make-local-variable 'xref-prompt-for-identifier)
  (setq xref-prompt-for-identifier nil)
  (add-hook 'xref-backend-functions #'phpactor-xref-backend nil t))

phpactor.el Outdated
@@ -821,5 +832,8 @@ function."
(let ((arguments (phpactor--command-argments :source :path :offset)))
(apply #'phpactor-action-dispatch (phpactor--rpc "change_visibility" arguments))))

(when phpactor-use-xref
(require 'phpactor-xref))
Copy link
Member

Choose a reason for hiding this comment

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

Can replace (when (require ...)) with autoload.

(autoload 'phpactor-xref-backend "phpactor-xref"
  "Phpactor backend for Xref.")

phpactor-xref.el Outdated
;; Created: 05 Aug 2019
;; Version: 0.1.0
;; Keywords: tools, php
;; Package-Requires: ((phpactor "0.1.0"))
Copy link
Member

Choose a reason for hiding this comment

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

Please add (seq "2") into Package-Requires.

This is because Phpactor targets Emacs 24.4 and may not include the seq package.

;; xref backend using Phpactor.

;;; Code:
(require 'xref)
Copy link
Member

Choose a reason for hiding this comment

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

Please add (require 'seq).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for the review !

@kermorgant kermorgant changed the base branch from feature/phpactor-executable-defcustom-take2 to master August 12, 2019 15:38
phpactor-xref.el Outdated
(xref-make (map-elt candidate 'file)
(xref-make-file-location (map-elt candidate 'file)
(map-elt candidate 'line)
0)))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

having 0 here for the column is annoying as we already have the point position from the start of file.

(push (list (cons 'file path)
(cons 'line (plist-get reference :line_no))
;; (cons 'symbol symbol)
;; (cons 'match match)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe worth trying to fetch content of line at :line_no, which could permit to get a preview while listing candidates in minibuffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that was already given using :line

@kermorgant
Copy link
Contributor Author

fixed 2 small thing regarding references but still need to make find-definition behave correctly


(defun phpactor--xref-find-definitions ()
"Find definitions or jump directly when only one found."
(phpactor-goto-definition))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this works but with a mysterious Wrong type argument: listp 2070

kermorgant and others added 2 commits August 25, 2019 17:44
Thanks Nicolas Petton for inspiring this with your work on xref-js2.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants