|
From: Karthik C. <kar...@gm...> - 2021-01-18 04:53:53
|
Hi Eric,
The `matlab-local-xref` backend is quite useful, thank you for taking
the time to go through it. I did notice something in this function:
(defun matlab-shell-xref-activate ()
"Function to activate xref backend.
Add this function to `xref-backend-functions' for matlab shell to
use xref to find function and variable definitions."
(and (member major-mode '(matlab-mode matlab-shell-mode org-mode))
(matlab-shell-active-p)
'matlab-shell))
The test for major-mode is unnecessary since the xref backend is only
loaded when matlab-mode is activated. Even if you decide to leave it in
the reference to `org-mode' is completely superfluous. This is some
debug code that I forgot to take out before I submitted the patch. (The
same applies to the function `matlab-local-xref-activate'.) So we just
need
(defun matlab-shell-xref-activate ()
"Function to activate xref backend.
Add this function to `xref-backend-functions' for matlab shell to
use xref to find function and variable definitions."
(and (matlab-shell-active-p)
'matlab-shell))
Karthik
Eric Ludlam <eri...@gm...> writes:
> Hi Karthik,
>
> Thanks for the update and explanation.
>
> I finally had some time to sit with your code, try it out in a few
> situations, and teach myself how xref works. I haven't had much
> opportunity to learn all the new stuff in eieio that it uses, so that
> was nice.
>
> I created matlab-xref.el on the shellcomplete branch which includes a
> tweaked version of your code.
> I included an xref engine that will do local symbol lookups for local
> functions and the like. Combined, with the shell version, you can get
> to most places quickly.
>
> I think this is a pretty good start. I've already found it changed the
> way I do navigation. Very nice.
>
> Eric
>
>
> On 12/23/20 5:04 AM, Karthik Chikmagalur wrote:
>> Hi Eric,
>>
>>> I did look at the elisp backend which it advertised as the prime
>>> example. That one is quite large compared to your patch. Do you
>>> think the below would expand out to be much larger? I'm considering
>>> if xref support would move into it's own file or not.
>> The xref support for MATLAB in my patch is very short (relatively)
>> because
>>
>> - I wrote it as a wrapper around `matlab-shell-which-fcn', which does
>> the heavy lifing of finding the function in question. Matlab-shell
>> already has support for jumping to definitions so I reused it. The
>> downside is that matlab-shell needs to be running for xref support to
>> work. This is not ideal, but the alternative is to write code to find
>> function definitions from scratch.
>>
>> - Only jump to definition is implemented. Two other xref functions:
>> `xref-backend-apropos' and `xref-find-references', are not implemented
>> at all. I want to look into adding these, but I'm not sure right now
>> how to go about it.
>>
>>> I also note that the elisp version doesn't use require for xref, and
>>> your patch depends on the feature existing when the code loads. I
>>> think it does so via:
>>>
>>> (declare-function xref-make "xref" (summary location))
>>>
>>> Will that work here too?
>> I think `declare-function' keeps the byte-compiler from complaining and
>> doesn't do much else. This should help here, though, and checking if
>> xref is loaded should be unnecessary.
>>
>> In summary, a slightly modified version of this patch can be included as
>> preliminary xref support for matlab-mode. It can be its own file if it's
>> expected to be expanded in the future with `xref-find-references' and
>> other functionality, and prehaps written in a way as to not require
>> matlab-shell to be running.
>>
>> Attached is a patch (as a separate file) with some minor modifications
>> from last time. For it to be integrated into matlab-mode, one other
>> piece of configuration is needed:
>>
>> (add-hook 'xref-backend-definitions #'matlab-shell-xref-activate)
>>
>> This needs to be added in the matlab-mode major mode definition in
>> `define-derived-mode matlab-mode' in matlab.el.
>>
>> Karthik
>>
|