Thread: [Distel-hackers] who_calls rewrite
Status: Beta
Brought to you by:
lukeg
From: Matthias R. <mat...@so...> - 2007-09-02 16:12:14
Attachments:
who_calls.patch
|
Hi, I found distel's "who calls" (C-c C-d w) feature rather cumbersome to use. It takes ages to start the first time and then doesn't update when the code changes. Plus the code for it is quite horrible and brittle. So I replaced all ~250 lines of it with a single call to xref:q/2. See the attached patch. Invoking "who calls" for the first time still takes a while, but less than previously. And code changes are tracked properly. Regards, Matthias PS: xref could also replace some of the other code in distel.erl, such as the module/function completion support. |
From: Bill C. <bil...@gm...> - 2007-09-03 17:47:43
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > I found distel's "who calls" (C-c C-d w) feature rather cumbersome to > use. It takes ages to start the first time and then doesn't update when > the code changes. Plus the code for it is quite horrible and brittle. > > So I replaced all ~250 lines of it with a single call to xref:q/2. See > the attached patch. > > Invoking "who calls" for the first time still takes a while, but less > than previously. And code changes are tracked properly. Very nice. This is much faster and (with my limited testing) seems to produce the same results. I've committed your patch. - Bill |
From: Matthias R. <mat...@so...> - 2007-09-03 18:08:08
|
Bill, Bill Clementson wrote: > Very nice. This is much faster and (with my limited testing) seems to > produce the same results. I've committed your patch. Cheers. On a related note, who_calls actually returns the line number of the call sites but this currently isn't displayed/used. This was the case in the previous version too. There is a comment in the code saying "FIXME: Use line number for a hyperlink.". I reckon one way to implement something sensible along these lines is to construct an, in Emacs-lingo, "overlay" for the displayed m:f/a with the line number information. We can then look for this invisible piece of information in "M-." (aka erl-find-source-under-point) and, if present, navigate to it. Any other suggestions? Matthias. |
From: Bill C. <bil...@gm...> - 2007-09-03 18:44:28
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > Bill Clementson wrote: > >> Very nice. This is much faster and (with my limited testing) seems to >> produce the same results. I've committed your patch. > > Cheers. > > On a related note, who_calls actually returns the line number of the > call sites but this currently isn't displayed/used. This was the case > in the previous version too. > > There is a comment in the code saying "FIXME: Use line number for a > hyperlink.". I reckon one way to implement something sensible along > these lines is to construct an, in Emacs-lingo, "overlay" for the > displayed m:f/a with the line number information. I thought that "overlays" were more for display properties and that "text-properties" would be what you would want to use for storing the line# and source file of any matches. > We can then look for > this invisible piece of information in "M-." (aka > erl-find-source-under-point) and, if present, navigate to it. > > Any other suggestions? Are you suggesting that "M-." be used to go to the "call"? Normally, "M-." is used to go to the definition, not a reference. Why not just use "RET"? You could have a separate RET key binding for the who-calls results buffer. - Bill |
From: Matthias R. <mat...@so...> - 2007-09-03 19:00:32
|
Bill Clementson wrote: >> There is a comment in the code saying "FIXME: Use line number for a >> hyperlink.". I reckon one way to implement something sensible along >> these lines is to construct an, in Emacs-lingo, "overlay" for the >> displayed m:f/a with the line number information. > > I thought that "overlays" were more for display properties and that > "text-properties" would be what you would want to use for storing the > line# and source file of any matches. If you say so. I am not an emacs hacker. > Are you suggesting that "M-." be used to go to the "call"? Normally, > "M-." is used to go to the definition, not a reference. Why not just > use "RET"? You could have a separate RET key binding for the who-calls > results buffer. That would be better indeed. It still has to do much of the same work as "M-.", so I was just trying to cut some corners. Any chance you could have a go at implementing this? Matthias. |
From: Bill C. <bil...@gm...> - 2007-09-03 19:09:43
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > Bill Clementson wrote: >>> There is a comment in the code saying "FIXME: Use line number for a >>> hyperlink.". I reckon one way to implement something sensible along >>> these lines is to construct an, in Emacs-lingo, "overlay" for the >>> displayed m:f/a with the line number information. >> >> I thought that "overlays" were more for display properties and that >> "text-properties" would be what you would want to use for storing the >> line# and source file of any matches. > > If you say so. I am not an emacs hacker. > >> Are you suggesting that "M-." be used to go to the "call"? Normally, >> "M-." is used to go to the definition, not a reference. Why not just >> use "RET"? You could have a separate RET key binding for the who-calls >> results buffer. > > That would be better indeed. It still has to do much of the same work > as "M-.", so I was just trying to cut some corners. > > Any chance you could have a go at implementing this? Not in the short term. I've taken a bit of a break from Erlang due to other commitments. It shouldn't be a difficult hack though, so maybe someone else would like to take it on? - Bill |
From: Matthias R. <mat...@so...> - 2007-09-18 04:02:50
|
Bill, Bill Clementson wrote: > I had a bit of free time today, so I implemented this (patch > attached). The who-calls buffer will now let you press RET on a line > and it will open the source member of the module containing the > caller and jump to the line number of the function definition of the > caller (which is what is returned by your "(Lin)(domain (E || ~p))" > who-calls xref query. Great. > I think it would be preferable to jump to the place in the function > where the call was made; however, I don't know what the correct xref > string should be. It looks like changing the "(Lin)" to something > like "(LLin + XLin)" is what is needed (so that xref returns the line > numbers for both local and external calls). However, I played around > with some variations and couldn't figure out the right one. And, the > xref documentation didn't provide any enlightenment. Do you know what > the xref query should be changed to? Try something like {ok, Calls} = xref_q(...,"(XXL)(Lin)(E || ~p)", [MFA]), lists:flatten([[{Caller, Line} || Line <- Lines] || {{{Caller,_},_}, Lines} <- Calls]). Lines is a list since there can be several call sites per function. In the above I am just flatting that info, but depending on how you want to represent this in the UI you may want to do something different. Matthias. |
From: Matthias R. <mat...@so...> - 2007-09-18 08:00:30
|
Bill, Bill Clementson wrote: > Yep, that did it - thanks! Works for me too. Thanks for implementing this change. > Incidentally, I probably mucked around trying to figure out the xref > query string to use for longer than it took me to write the elisp > code. Is there any better documentation or examples of xref other > than the tools xref document? e.g. - better than: > http://www.erlang.org/doc/man/xref.html There is the User Guide at http://www.erlang.org/doc/apps/tools/xref_chapter.html#5, but it's not all that much more helpful. I am not aware of any other documentation / tutorials / examples. I found that carefully reading the above two documents from start to finish, and trying a few self-constructed examples along the way, helped quite a bit. I suspect the inaccessibility of the xref documentation is one of the reasons it isn't known and used more widely. Somebody should write a few introductory blog posts ... Matthias. |
From: Bill C. <bil...@gm...> - 2007-09-18 13:44:34
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > Bill Clementson wrote: >> Yep, that did it - thanks! > > Works for me too. Thanks for implementing this change. > >> Incidentally, I probably mucked around trying to figure out the xref >> query string to use for longer than it took me to write the elisp >> code. Is there any better documentation or examples of xref other >> than the tools xref document? e.g. - better than: >> http://www.erlang.org/doc/man/xref.html > > There is the User Guide at > http://www.erlang.org/doc/apps/tools/xref_chapter.html#5, but it's not > all that much more helpful. Yes, I saw that one too (it's linked to from the above page). > I am not aware of any other documentation / tutorials / examples. > > I found that carefully reading the above two documents from start to > finish, and trying a few self-constructed examples along the way, > helped quite a bit. > > I suspect the inaccessibility of the xref documentation is one of the > reasons it isn't known and used more widely. I did google around a bit yesterday trying to find examples as I thought someone had surely been banging their head against the wall with xref before. Surprisingly, I didn't find much. I suspect you're right - the documentation probably puts most people off. > Somebody should write a > few introductory blog posts ... Hmm, actually, that's a really good idea and I know the ideal person for the job! LShift has a blog with a growing collection of erlang-related posts - a great forum for you to share your xref knowledge! ;-) Thanks again for your work in getting xref into distel. - Bill |
From: Bill C. <bil...@gm...> - 2007-09-03 18:46:28
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > Bill Clementson wrote: > >> Very nice. This is much faster and (with my limited testing) seems to >> produce the same results. I've committed your patch. > > Cheers. > > On a related note, who_calls actually returns the line number of the > call sites but this currently isn't displayed/used. This was the case > in the previous version too. > > There is a comment in the code saying "FIXME: Use line number for a > hyperlink.". I reckon one way to implement something sensible along > these lines is to construct an, in Emacs-lingo, "overlay" for the > displayed m:f/a with the line number information. I thought that "overlays" were more for display properties and that "text-properties" would be what you would want to use for storing the line# and source file of any matches. > We can then look for > this invisible piece of information in "M-." (aka > erl-find-source-under-point) and, if present, navigate to it. > > Any other suggestions? Are you suggesting that "M-." be used to go to the "call"? Normally, "M-." is used to go to the definition, not a reference. Why not just use "RET"? You could have a separate RET key binding for the who-calls results buffer. - Bill |
From: Torbjorn T. <to...@to...> - 2007-09-03 18:58:35
|
Matthias Radestock wrote: > Hi, > > I found distel's "who calls" (C-c C-d w) feature rather cumbersome to > use. It takes ages to start the first time and then doesn't update when > the code changes. Plus the code for it is quite horrible and brittle. > > So I replaced all ~250 lines of it with a single call to xref:q/2. See > the attached patch. > > Invoking "who calls" for the first time still takes a while, but less > than previously. And code changes are tracked properly. Excellent! However, it seems like when there are a large amount of callers I get nothing displayed!? Cheers, Tobbe |
From: Matthias R. <mat...@so...> - 2007-09-03 19:02:17
|
Tobbe, Torbjorn Tornkvist wrote: > it seems like when there are a large amount of callers > I get nothing displayed!? That's strange. Two questions: 1) can you give me an example so I can try reproducing this? 2) what is the behaviour of the previous version? Matthias. |
From: <to...@to...> - 2007-09-03 19:22:14
|
Matthias Radestock wrote: > Tobbe, > > Torbjorn Tornkvist wrote: >> it seems like when there are a large amount of callers >> I get nothing displayed!? > > That's strange. > > Two questions: > > 1) can you give me an example so I can try reproducing this? I'm afraid not, since it the code isn't open source. Hm...but now when I tried it again, it seems to work... So I take back my earlier statement. It is now just excellent :-) Cheers, Tobbe |
From: Bill C. <bil...@gm...> - 2007-09-18 05:09:42
|
Hi Matthias, Matthias Radestock <mat...@so...> writes: > Bill Clementson wrote: >> I had a bit of free time today, so I implemented this (patch >> attached). The who-calls buffer will now let you press RET on a line >> and it will open the source member of the module containing the >> caller and jump to the line number of the function definition of the >> caller (which is what is returned by your "(Lin)(domain (E || ~p))" >> who-calls xref query. > > Great. > >> I think it would be preferable to jump to the place in the function >> where the call was made; however, I don't know what the correct xref >> string should be. It looks like changing the "(Lin)" to something >> like "(LLin + XLin)" is what is needed (so that xref returns the line >> numbers for both local and external calls). However, I played around >> with some variations and couldn't figure out the right one. And, the >> xref documentation didn't provide any enlightenment. Do you know what >> the xref query should be changed to? > > Try something like > {ok, Calls} = xref_q(...,"(XXL)(Lin)(E || ~p)", [MFA]), > lists:flatten([[{Caller, Line} || Line <- Lines] > || {{{Caller,_},_}, Lines} <- Calls]). > > Lines is a list since there can be several call sites per function. In > the above I am just flatting that info, but depending on how you want > to represent this in the UI you may want to do something different. Yep, that did it - thanks! Incidentally, I probably mucked around trying to figure out the xref query string to use for longer than it took me to write the elisp code. Is there any better documentation or examples of xref other than the tools xref document? e.g. - better than: http://www.erlang.org/doc/man/xref.html I've committed the changes. - Bill |