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 <ericlud...@gmail.com> 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
>>


_______________________________________________
Matlab-emacs-discuss mailing list
Matlab-emacs-discuss@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/matlab-emacs-discuss

Reply via email to