scala-ide / scala-search

Next Scala Search Engine
6 stars 10 forks source link

Eclipse command for finding occurrence of selection #36

Closed mads-hartmann closed 11 years ago

mads-hartmann commented 11 years ago

This commit provides an Eclipse command for finding all occurrences of the Scala entity that is selected in the current editor.

Currently just supports methods

Fixes #1001686

skyluc commented 11 years ago

I think it would be good to add the action in the editor context menu too. It is a bit simpler to find than guessing the key shortcut. Example on how to do it could be done can be found in the Play2 plugin.

skyluc commented 11 years ago

One main thing I am missing are the markers of the occurrences in the editors. With the Java search, the occurrences are marked with a yellow arrow, and a different background color.

I don't know if it should be part of this pr, or if it is part of a different ticket. I would expect it to be done by the EditorMatchAdapter, but I can be wrong.

mads-hartmann commented 11 years ago

@skyluc Regarding the markers: They're currently being set by Eclipse when you run the handler. I reckon it's one of the subclass I extend that takes care of it :)

mads-hartmann commented 11 years ago

@skyluc About adding a context menu, what is the locationUri? Do we have an ID for the Scala editor or do I need to use the Java?

skyluc commented 11 years ago

I had trouble with it in the play2 plugin, the doc is not really explicit. I was tempted to say scala.tools.eclipse.ScalaSourceFileEditor.EditorContext?before=additions, but looking at the plugin.xml from scala-ide, it looks like there is a context defined for compilation units.

mads-hartmann commented 11 years ago

@skyluc

screenshot on 2013-05-22 at 15 19 47

mads-hartmann commented 11 years ago

@skyluc Just pushed a couple of commits. I believe I've addressed all your comments :)

skyluc commented 11 years ago

Except that I am not seeing the search results markers in the editors, it is working fine. I can't wait to get more :smile:.

dragos commented 11 years ago

It works well, and it's super-fast! Well done! I filed a few tickets for the issues I've seen, but they should not stop this PR from merging!

dragos commented 11 years ago

Besides a couple of minor points, this looks really good to me! :+1:

mads-hartmann commented 11 years ago

Okay, so this tests fails sometimes because we have an upper-bound on the timeout. This will get fixed in https://scala-ide-portfolio.assembla.com/spaces/scala-ide/tickets/1001724 so it can be ignored for now.

skyluc commented 11 years ago

On my markers problem: they appear correctly when I switch editors, but not at first just after invoking the command.

dotta commented 11 years ago

Besides some minor aesthetical comments, this looks really good to me - and works very nicely!

Feel free to squash commits as needed and merge.

mads-hartmann commented 11 years ago

@skyluc @dotta Yes, if you search, switch to another file and then back then they will show. So I think @dotta was right and we simply need to "refresh" the editor in some what. Any ideas?

mads-hartmann commented 11 years ago

Saving the marker issue for later. Merged. Thanks for the review everyone!