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

xref-backend for R mode with etags? #686

Closed
wenjie2wang opened this issue Sep 18, 2018 · 8 comments
Closed

xref-backend for R mode with etags? #686

wenjie2wang opened this issue Sep 18, 2018 · 8 comments

Comments

@wenjie2wang
Copy link

wenjie2wang commented Sep 18, 2018

Hello, I found ESS has a xref-backend ess-r-xref-backend for finding definitions and it would be enabled for Emacs 25+. When using Emacs < 25, I am able to find definitions or tags by M-x visit-tags-table a TAGS file and M-.. However, I find that it does not work for Emacs 25+ by default.

A simple solution I find is to enable the etags backend by M-x xref-etags-mode. It allows me to "jump to definitions" again. However, I think I had to run M-x xref-etags-mode for every R file I would like to "jump" from. Is there anything I missed?

If so, I think it would be great if the functionality provided by find-tag (from etags.el) for Emacs < 25 still works "out-of-box" when using Emacs 25+. A possibly related workaround I find is Pan Xie's example at EmacsWiki: https://www.emacswiki.org/emacs/TagsFile. I did some quick modifications as follows:

;;; enable etags backend in addition to ess-r-xref-backend
;;; modified from Pan Xie's example at https://www.emacswiki.org/emacs/TagsFile
(when (require 'xref nil t)
  (defun xref--find-xrefs (input kind arg display-action)
    "Re-define the `xref--find-xrefs'.
 This is a re-defined version of `xref--find-xrefs'.  This
 function will call all backends until the definitions are found
 or the `xref-backend-functions' is exhausted."
    (let ((fn (intern (format "xref-backend-%s" kind)))
          (tail xref-backend-functions))
      (cl-block nil
        (while tail
          (let* ((backend-fn (car tail))
                 (backend (and (symbol-function backend-fn) (funcall backend-fn)))
                 (xrefs (and backend (funcall fn backend arg))))
            (when xrefs
              (cl-return (xref--show-xrefs xrefs display-action))))
          (setq tail (cdr tail)))
        (user-error "No %s found for: %s" (symbol-name kind) input)))))

;; add a hook function to the `ess-mode-hook':
(defun hook:ess-mode ()
  "Hook for ess-mode."
  (when (require 'xref nil t)
    (setq-local xref-backend-functions '(etags--xref-backend ess-r-xref-backend t))))
(add-hook 'ess-mode-hook 'hook:ess-mode)

Will it be a good idea to similarly add etags to the xref-backend for R mode? Thanks!

@wenjie2wang wenjie2wang changed the title xref-backend for R model xref-backend for R mode with etags? Sep 18, 2018
@vspinu
Copy link
Member

vspinu commented Sep 18, 2018

Do I understand correctly that your problem is that M-. executes our R-xref and doesn't propagate to etags backend as it should in theory?

xref-backend-functions is a variable defined in ‘xref.el’.
Its value is (elisp--xref-backend t)
Local in buffer ess-r-xref.el; global value is 
(etags--xref-backend)

The following works but is not as effient as it could be because xref-backend-definitions is called twice (likely a bug in xref):

(defun ess-r-xref-backend ()
  (let ((sym (xref-backend-identifier-at-point 'ess-r)))
    (when (and sym (xref-backend-definitions 'ess-r sym))
      'ess-r)))

(defvar ess-r--xref-cache nil)
(cl-defmethod xref-backend-definitions ((_backend (eql ess-r)) symbol)
  (if ess-r--xref-cache
      (prog1 ess-r--xref-cache
        (setq ess-r--xref-cache nil))
    (let ((xref (ess-r-xref--xref symbol)))
      (when xref
        (setq ess-r--xref-cache (list xref))))))

I am not sure we want to promote this though. One major limitation to this way is that the set of all R completions can no longer be shown if no identifier has been found at point. I believe this is a big deal.

As to my mind the optimal solution is to have a dedicated key for etags (I use M-g M-t) and be done with such issues once and for all.

@wenjie2wang
Copy link
Author

Thanks for looking into this issue.

Yes, I expected M-. to find definitions from a TAGS file as it used to in Emacs < 25. When using Emacs 25+, xref-backend-functions is (ess-r-xref-backend t) when ESS mode is loaded. The M-. needs an inferior ESS process and cannot find tags/definitions from a TAGS file unless they are loaded by e.g. devtools::load_all().

As to my mind the optimal solution is to have a dedicated key for etags (I use M-g M-t) and be done with such issues once and for all.

It makes sense to me. Thanks.

@JoshOBrien
Copy link

JoshOBrien commented Mar 20, 2019

@wenjie2wang Thanks for pasting in the code that worked for you. I too have been frustrated at the change of behavior of M-. in *.R files that I long relied on, and adding your code to my .emacs seems to have restored the old behavior.

Unfortunately (and surely because I don't understand what's going on 'under the hood'), I didn't understand this bit of the post by @vspinu

As to my mind the optimal solution is to have a dedicated key for etags (I use M-g M-t) and be done with such issues once and for all.

Would one of you mind including the code that I'd need to bind the old functionality (or that of a direct invocation of the etags search functionality) to M-g M-t? It sounds (?) like both of you agreed that'd be a cleaner solution than the code you added.

Also, am I correct in understanding that the change in behavior was a by-product of the code modifications involved in the effort to automatically produce tags-like behavior for any package loaded/sourced via a call to devtools::load_all()?

Thanks!

@vspinu
Copy link
Member

vspinu commented Mar 21, 2019

I have this

(defun my-xref-find-etags ()
  (interactive)
  (let ((xref-backend-functions '(etags--xref-backend))
        (thing (xref--read-identifier "Find definitions of: ")))
    (xref--find-definitions (replace-regexp-in-string "^\\.\\|\\.$\\|^[^/]+/" "" thing) nil)))

(global-set-key (kbd "M-g M-t") 'my-xref-find-etags)

I think there is a simpler built-in solution. I used those internals because I want to simplify the identifier at point in order to maximize the matches which I get at point.

@JoshOBrien
Copy link

@vspinu Thanks. That works nicely, albeit: (1) with a ~three second delay each time; (2) always asking for confirmation that I want to search for the symbol at point; and (3) still forcing a launch of an R instance before the first tag-lookup.

Thanks also for all the work you've done to develop and improve ESS!

@nverno
Copy link
Contributor

nverno commented Mar 21, 2019

@JoshOBrien It's strange there should be a delay, and shouldn't force launching an R instance. Have you created a TAGS file with your tags program (etags, ctags, etc.)? If you don't want to be prompted you could use something like

(defun my-xref-find-etags ()
  (interactive)
  (let* ((xref-backend-functions '(etags--xref-backend))
         (thing (xref-backend-identifier-at-point 'etags)))
    (xref-find-definitions-other-window thing)))

@vspinu
Copy link
Member

vspinu commented Mar 22, 2019

Indeed. There is nothing R related in that code, so (3) should not happen. The delay is because I want to be asked for the identifier and it takes time to collect candidates on big etags. @nverno's solution is superior and should better cater to your needs.

@JoshOBrien
Copy link

@vspinu (3) was unexpected for me as well, but I can confirm (having just tried it again) that it does happen, at least with my setup. Fortunately, it does not happen when I use the code @nverno supplied, so I'll just go with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants