Bugzilla – Full Text Bug Listing
| Summary: | java class hyperlink detection should provide multiple options when multiple results are found for a class name | ||
|---|---|---|---|
| Product: | z_Archived | Reporter: | David Green <greensopinion> |
| Component: | Mylyn | Assignee: | Jingwen 'Owen' Ou <jingweno> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | jingweno, steffen.pingel |
| Version: | dev | ||
| Target Milestone: | 3.1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Bug Depends on: | 244380 | ||
| Bug Blocks: | 239087 | ||
| Attachments: | |||
|
Description
David Green
Does a multiple hyperlinks make sense for this, I mean the new 3.4 feature? Yes, I think that would be a good solution. Mylyn currently supports Eclipse 3.3 as well as 3.4 though so this would have to be implemented through reflection or by back-porting the 3.4 hyperlink presenter. The downside of using multiple hyperlinks is that the Java search has to be done in the hyperlink detector rather than in the hyperlink -- which means that the penalty is paid even if the user doesn't click on the link. This could be a major problem for editors that have dozens of comments with class references. I like the idea of it working more like the CTRL+SHIFT+T (Open Type) functionality of eclipse, where the search is done at the time of clicking as it does now. Another compromise that I can come up with its to drop the absolute path of a IJavaElement, e.g. org.foo.bar.ITask. (In reply to comment #3) > which means that the > penalty is paid even if the user doesn't click on the link. This could be a > major problem for editors that have dozens of comments with class references. I think we are already searching the Java class to minimize false positive, e.g. JavaResourceHyperlinkExtension.isResourceExists(..). Maybe its not a good idea to do that, any comments to improve the performance as well as the flexibility of the extension point, e.g. bug 240423? That's a very good point David. I agree that delaying the processing until after the link is clicked is better. As Owen points out JavaResourceHyperlinkExtension.isResourceExists() already does some processing though. Owen, does it make sense to not do that in the case of Java where we don't know how long it will take? To present multiple matches we could popup a menu in case there are multiple matches similar to the 3.4 hyperlink presenter but I am not sure how well the user interaction would work if searching for the class takes a while. (In reply to comment #4) > Another compromise that I can come up with its to drop the absolute path of a > IJavaElement, e.g. org.foo.bar.ITask. I do like the short form. I think I would even prefer if it matched on just "class" or "java". Would it be possible to drop the full qualified name when a modifier is pressed, i.e. it's a copy rather than a move operation? Created attachment 110182 [details]
a patch pops up a dialog for multiple hyperlinks
Created attachment 110183 [details]
mylyn/context/zip
Created attachment 110184 [details]
screenshot of the popup dialog
(In reply to comment #5) > Owen, does it make sense to not do that in the case of Java where we > don't know how long it will take? I added a patch in bug 244380 to fix it. > To present multiple matches we could popup a menu in case there are multiple > matches similar to the 3.4 hyperlink presenter but I am not sure how well the > user interaction would work if searching for the class takes a while. I used a popup dialog in my last patch. I like it! You can try if it fits. > I do like the short form. I think I would even prefer if it matched on just > "class" or "java". Would it be possible to drop the full qualified name when a > modifier is pressed, i.e. it's a copy rather than a move operation? I am not sure I totally understand this point. Its possible to drop the full qualified name to the task editor. > > I do like the short form. I think I would even prefer if it matched on just
> > "class" or "java". Would it be possible to drop the full qualified name when a
> > modifier is pressed, i.e. it's a copy rather than a move operation?
>
> I am not sure I totally understand this point. Its possible to drop the full
> qualified name to the task editor.
I was suggesting to either paste the short name or the full qualified name depending on the dnd operation. It seems that the package explorer deletes the source file if I a use move operations (rather than copy) so it might be best to only support copy.
> I was suggesting to either paste the short name or the full qualified name
> depending on the dnd operation. It seems that the package explorer deletes the
> source file if I a use move operations (rather than copy) so it might be best to
> only support copy.
It won't. Please take a look at TaskEditorDropTarget.dragEnter(..). It will convert all move operations to copy.
Maybe it's a Linux thing. If I press shift when dropping a file from the package explorer it's deleted from the local filesystem. Created attachment 110190 [details]
a patch disables converting move to copy in TaskEdtiorDropTarget
ok, lets disalbe it for now. Actaully it works ok on my machine (window xp)
I have applied the first patch. Thanks! I have changed the drop target implementation to always use copy. This works on Linux as well. Owen, as per conference call can you check if it is possible to improve the implementation by using the Java Open Type dialog? Created attachment 110763 [details]
a patch that uses Java Open Type dialog whne clicking the hyperlink
I am not sure whether its a good idea to use open type dialog. But it does have many false positives.
when I clicked "java class HTML", it displays HTMLParagraphElement as the first element, and other elements starting with HTML* fill the dialog. Excellent! Patch applied. The dialog does indeed show all prefix matches which may include unwanted classes. Given that the perfect matches are on top of the list I think the benefits of the richer dialog outweigh the disadvantages. Created attachment 113061 [details]
mylyn/context/zip
|