From: Eric L. <eri...@gm...> - 2021-01-18 16:59:59
|
Thanks Karthik, I made that change too and pushed onto the branch. Eric On 1/17/21 11:53 PM, Karthik Chikmagalur wrote: > 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 >>> > . |
From: Uwe B. <ou...@ma...> - 2021-01-18 08:49:12
Attachments:
smime.p7s
|
>>> "KC" == Karthik Chikmagalur <kar...@gm...> writes: > 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 Sorry for jumping into the discussion (and almost knowing nothing about xref), I do use org mode to execute matlab code via babel and I am sure other users as well. Does this in anyway influence the part > (and (member major-mode '(matlab-mode matlab-shell-mode org-mode)) You were just talking about? |
From: Karthik C. <kar...@gm...> - 2021-01-18 09:29:03
|
> Sorry for jumping into the discussion (and almost knowing nothing about > xref), I do use org mode to execute matlab code via babel and I am sure > other users as well. Me too, that's why I put it there while I was testing the patch. > Does this in anyway influence the part > >> (and (member major-mode '(matlab-mode matlab-shell-mode org-mode)) > > You were just talking about? That is indeed the line I suggested removing. However it's unnecessary, since just including it doesn't let us use xref in org-mode either. For that we need something like this in `org-mode-hook' instead: (add-hook 'org-mode-hook (lambda () (add-hook 'xref-backend-functions 'matlab-local-xref-activate nil t) (add-hook 'xref-backend-functions 'matlab-shell-xref-activate nil t))) The matlab-shell based xref backend will fail to jump to definitions in org-babel code blocks, though it will still work on built-ins and functions defined in dedicated m-files. The local xref-backend should fill in the gaps there, although I haven't tested it yet. Karthik |
From: Uwe B. <ou...@ma...> - 2021-01-18 17:06:14
Attachments:
smime.p7s
|
>>> "KC" == Karthik Chikmagalur <kar...@gm...> writes: >> Sorry for jumping into the discussion (and almost knowing nothing about >> xref), I do use org mode to execute matlab code via babel and I am sure >> other users as well. > Me too, that's why I put it there while I was testing the patch. >> Does this in anyway influence the part >> >>> (and (member major-mode '(matlab-mode matlab-shell-mode org-mode)) >> >> You were just talking about? > That is indeed the line I suggested removing. However it's unnecessary, > since just including it doesn't let us use xref in org-mode either. For > that we need something like this in `org-mode-hook' instead: > (add-hook 'org-mode-hook > (lambda () > (add-hook 'xref-backend-functions 'matlab-local-xref-activate nil t) > (add-hook 'xref-backend-functions 'matlab-shell-xref-activate nil t))) Ok, good to know (although I am not a big fan of putting lambda functions into hooks since they are somehow difficult to remove without restarting emacs) > The matlab-shell based xref backend will fail to jump to definitions in > org-babel code blocks, though it will still work on built-ins and > functions defined in dedicated m-files. The local xref-backend should > fill in the gaps there, although I haven't tested it yet. Shall I start testing it or does this code still need some polishing? |
From: Haik S. <hai...@ku...> - 2021-01-19 17:34:28
|
Hi Eric, I checked that `comint-prompt-read-only` is set to t, I guess that's it. In principle it is nice that you cannot delete the shell output, I remember long time ago when I was running shells in emacs I was annoyed that you could do it. Indeed, this problem can turn up in many places, so this hack is not ideal. In the documentation it is written that the correct way to temporarily override the read-only-ness is to use `comint-kill-region` instead of `kill-region` etc. Best regards and thanks a lot for all the great code! Haik 17.01.2021 21:22 Eric Ludlam <eri...@gm...> kirjutas: > Thanks Haik. > > That implies a patch like this will fix the issue... ? > > Do you know what is being set to read-only and why? A better > patch might > disable whatever that is since other code may need something > similar, and it > would be better to fix the source than add little patches like > this around. > > Eric |