Community
Participate
Working Groups
Eclipse 3.1M5a There should be a way to add own IHyperlinkDetectors to text editors, perhaps through an extension point. I'd like to be able to detect certain kinds of text in a document, then being able to open e.g. other editors when control clicking on that text. If there's a different way I can achieve this, feel free to add it as comment ;-)
Yes, we also think this makes sense. Would also need preference UI similar to hovers. >If there's a different way I can achieve this, feel free to >add it as comment ;-) Simplest scenario is to provide your own editor. The only thing you'd change is the source viewer configuration.
Thanks for the suggestion but I need to get this stuff into the existing Java editor. So I guess I need to wait for you to do the extension point... ;-))
I also have an interest in this extension point. My usecase is to allow hyperlinking to an issue tracker. For dodgey workarounds we usually put a marker in the code pointing to an issue so we can see this history of the fix.
Mylar has an IHyperlinkDetectors that links bugzilla reports from the Java editor, consistently with how the Bugzilla Web UI does it (e.g. bug 123). Currently adding it is a mess and has to be done with our own source configuration. So we would definitely like this extension point, probably specified per file extension or content type.
I imagine the extension point could associate a hyperlink detector to one of the following: -editor id -content type -partition type Best would be supporting all 3 :) Not only would I be interested in contributing my own hyperlink detectors, but I would also like to pick up others (ex: picking up java's hyperlink detectors in java content regions in jsps)
*** Bug 117485 has been marked as a duplicate of this bug. ***
Why limit it to editors?
Could you be a bit more concrete what you have in mind?
The console view does hyperlinks so could the same mechanism be used for that too? The one in the console view is a bit limited currently (see bug 120863).
>Why limit it to editors? The solution would most likely be per content type (i.e. IDocument.getContentType(...)) and allows text viewers to make use of it as well. Note that the text hyperlinks currently differ from the ones in the console by the fact that the text hyperlinks aren't permanent i.e. they are only rendered as link when the mouse moves over a hyperlink and the chosen modifier is pressed. The links in the console are always shown as link.
Is this something that's being considered for before the 3.2 freeze? Fyi for Mylar to add it's bug report hyperlink detector to .java we currently have the following hack in a subtype of JavaSourceViewerConfiguration which reads our own extension point for adding detectors. Per content type sound great to me, and might allow us to stop using a custom source viewer and editor. @Override public IHyperlinkDetector[] getHyperlinkDetectors(ISourceViewer sourceViewer) { if (!fPreferenceStore.getBoolean(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_HYPERLINKS_ENABLED)) return null; IHyperlinkDetector[] inheritedDetectors = super.getHyperlinkDetectors(sourceViewer); if (super.getEditor() == null) return inheritedDetectors; readDetectorsExtension(); if (hyperlinkDetectors == null) return inheritedDetectors; List<IHyperlinkDetector> detectors = new ArrayList<IHyperlinkDetector>(); detectors.addAll(Arrays.asList(inheritedDetectors)); detectors.addAll(hyperlinkDetectors); return detectors.toArray(new IHyperlinkDetector[hyperlinkDetectors.size()]); }
We will not be able to do this for 3.2 M5. Eventually we might add this after M5 since M5 freezes existing API but normally we still make API additions. Of course if an interested party sends in patches before M5 we'll try to look a those.
Created attachment 34057 [details] hyperlink detector extension Here is a first pass at patch for contributing hyperlink detectors to the Java editor. Although it is likely that this is not the right place to be doing extension reading, it made for the least intrusive and simplest patch. Let me know what you think and I'm happy to modify as necessary. Only testing done was manually checking that existing detectors remained, and that Mylar's contributed Bugzilla hyperlink detector worked.
Mik, the extension-point should be in a Platform Text plug-in. A good place would be 'org.eclipse.ui.workbench.texteditor'. We don't plan to add this to JDT UI only.
Created attachment 34118 [details] hyperlink detectors extension for org.eclipse.workbench.texteditor Here is the patch redone for org.eclipse.workbench.texteditor. As is, the extension will cause the hyperlink detector to be added to all AbstractTextEditor's (works for me, but others want it restricted to editor, content-type, or partition). I was tempted to add something similar to javaTypeCompletionProposalComputer: <partition type="__dftl_partition_content_type"/> <partition type="__java_string"/> But this is a pretty serious API decision that's out of my area of expertise, and currently the subtypes of IHyperlinkDetector have their own logic for dealing with partitions and it seems that converting those to the extension point should be considered.
Mik, I looked at the new patch. Here my comments: 1) The patch does the setup in the editor which is wrong. It should be done as before i.e. through the source viewer (configuration) and not mix them together. 2) Eat your own dog food: existing code needs to be rewritten to use the extension point. 3) The comment regarding being partition based is correct. This requires API changes and probably also appears in the extension point. 4) If you look for example at the Java hover extension point (org.eclipse.jdt.ui.javaEditorTextHovers) you can see some missing points in your patch: - should only read the extensions once (not each time an editor is opened) i.e. provide access through plug-in class - clients must be able to tell whether they should be loaded simply because they contribute a detector (see 'activate' attribute) - we definitely need a preference UI so that clients can control it We should also think about modifiers a bit more i.e. do we only provide a global one (currently Ctrl) which affects all detectors or should this be configurable? This would allow to resolve conflicts in cases where more than one detector handles the same element (see also Java > Editor > Hovers) preference page.
I'm leaning against configurable modifiers, having it all work via Ctrl, and resolving 'conflicts' by popping up a shell that allows the hyperlink to be selected. The interaction here seems similar to MS Office smart tags, which don't work well for other reasons, but we should be able to do better: 1) user sees text that looks like it could be a link (e.g. URL, nls tag, bug 123) 2) mouses over the text and presses Ctrl modifier, if 'conflict' present: - a hover-like shell comes up with all hyperlink proposals from all providers - each link has a text description of target and an icon specified by provider (i.e. to indicate the resource kind of the target) There is an issue with (1) though: for non obvious links users don't know when something is a hyperlink and nothing indicates this (e.g. how many know NLS tags are hyperlinks?). I know that at least in part for this reason few people are currently using the bug 123 hyperlink in Mylar (which is why this is a low priority item for us). What if we consider allowing the provider to specify that an affordance should be shown for the hyperlink? The affordance could be something like a blue version of the dashed smart-tag underline, and be off for the Java element detector.
>I'm leaning against configurable modifiers, having it all work via Ctrl, and >resolving 'conflicts' by popping up a shell that allows the hyperlink to be >selected. This has already been prepared but currently we do not provide an implementation, see IHyperlinkPresenter.canShowMultipleHyperlinks() HyperlinkManager.ALL detection strategy You could provide an IHyperlinkPresenter implementation that can handle multiple links. >What if we consider allowing the provider to specify >that an affordance should be shown for the hyperlink? While it would be nice to permanently render all hyperlinks in the editor it is much too expensive, not only for the NLS tags but also for the Java element navigation. Therefore we currently don't plan do offer this. If some links would be treated special it will even be harder for users to find other non-visible hyperlinks.
Right, I wasn't proposing rendering them for the Java element navigation, just making them an option for links that aren't obvious (not the primary structure being displayed, i.e. Java elements). Performance would not be the only problem with underlining all links--I've seen an editor with every Java element hyperlinked and it looks horrible. Unfortunatley only allowing some to hyperlink would be at the price of inconsistency so it seems fine to drop this for now. Regarding making further progress on this, I am happy to do more but have to cap the time I put into it over the next few weeks since this is a low priority item for Mylar (not sure about other clients). However, we've committed to not having a custom editor/sourceconfig anymore so we still have an interest in seeing this API. From comment#16 I can do (1), and should be able to do some or all of (4). I did (2) for my own code before posting the patch, but it's too involved for me to do that for the other hyperlink detectors, as is (3), so is that something that you guys are wiling to do?
Regarding to do 2): can't promise since our schedule is really tight. I'd like to see that being part of the patch because otherwise I first have to either do some work or load Mylar to see it working.
Hi, Any more on this being resolved? Our research group (Chisel @ Uvic) are very intrested in seeing a hyperlink extension point. At the moment we would like to add hyperlinkable tags to comment regions as part of a tool we are developing but because we dont want to go down the custom editor route, we have had to put this feature on hold..
The 3.2 train is rolling i.e. you'll have to wait for 3.3. Maybe Mik will provide another patch for this.
I believe that it is the repsonsibility of Platform to provide an extensible hyperlink detector mechanism, and while I was happy to provide the original patch it is considerably more work for me to change all of the existing hyperlink detectors in Platform as suggested. So I can only prioritize time for this based on Mylar user demand and there hasn't been much (most likely because nobody realized that we made bug reports hyperlinkable due to the lack of an affordance indicating the hyperlink, proposed solution to that is on comment#17). So for the reasons above we wrote this off for 3.2, but would really like to see it in 3.3, and if Platform/Text can't commit to it we could revisit our decision.
This is probably related to bug 153932. That bug is suggesting hyperlink detectors for the Team providers, but perhaps Platform would be the better place for this, so we could use same detectors/filters between editors, text panels (e.g. in history view), etc.
So, what needs to be done here in order to see those plugabble hyperlink detectors in 3.3M2? It does make sense to have global registry for those detectors, so they can be used in text editors as Mik initially suggested as well as in other tools, including Team/CVS plugin and 3rd party Team providers.
>So, what needs to be done here in order to see those plugabble hyperlink >detectors in 3.3M2? It is not on the M2 plan and hence the Text team won't work on this for M2 unless a good quality patch is provided - see my previous comments about what's missing from the initial patch.
(In reply to comment #26) > >So, what needs to be done here in order to see those plugabble hyperlink > >detectors in 3.3M2? > It is not on the M2 plan and hence the Text team won't work on this for M2 > unless a good quality patch is provided - see my previous comments about what's > missing from the initial patch. By the way, my primary interest in this issue is to get these hyperlinks available not only for editors but generally to any SourceViewers (e.g. to make them appear for CVS comments in History View). Are you sure that extension point placement to org.eclipse.ui.workbench.texteditor is actually right?
> but generally to any SourceViewers Just a reminder: the concept of plug-ins and extension points is not know at the JFace Text level. >Are you sure that extension >point placement to org.eclipse.ui.workbench.texteditor is actually right? That's where it will happen but feel free to provide a patch that follows/shows a better approach ;-)
(In reply to comment #28) > > but generally to any SourceViewers > Just a reminder: the concept of plug-ins and extension points is not know at > the JFace Text level. Sure. > >Are you sure that extension > >point placement to org.eclipse.ui.workbench.texteditor is actually right? > That's where it will happen but feel free to provide a patch that follows/shows > a better approach ;-) Apparently org.eclipse.team.cvs.ui plugin does have indirect dependency on org.eclipse.ui.workbench.texteditor trough StatusTextEditor in ShowAnnotationOperation class. So, other plugins may not use editors (see Ed's comment). So, maybe eclipse.ui or eclipse.ui.ide would be the better place for this extension point and the new API.
I biefly looked at Mik's patch and it looks like there is some gap in the Eclipse API. Current API for IHyperlinkDetector is: IHyperlink[] detectHyperlinks(ITextViewer textViewer, IRegion region, boolean canShowMultipleHyperlinks); This only allows to get instance of the document, but there is no reliable link between this document and some resource in the workspace. Even worse, most of the existing hyperlink detectors are installed into specific editor instance (see JavaElementHyperlinkDetector and PropertyKeyHyperlinkDetector). So, I wonder if IHyperlinkDetector API can be extended (I guess using a new interface IHyperlinkDetector2) to link using ither IPath or IResource: IHyperlink[] detectHyperlinks(ITextViewer textViewer, IRegion region, boolean canShowMultipleHyperlinks, IPath source); Then Mylar will be able to tell if "bug 123" is actually issue in Eclipse's bugzilla or bug in Mozilla's or Red Hat issue tracking system. There is also approach to require IDocument returned by textViewer to be adaptable for example to IPath, but I am not sure if that would be the the better idea...
>So, I wonder if IHyperlinkDetector API can be extended ( Yes. We often do this. It would be IHyperlinkDetectorExtension which would not inherit from IHyperlinkDetector. >There is also approach to require IDocument returned by textViewer to be >adaptable for example to IPath A document does and most not know its path hence such an adapter can't be implemented.
Created attachment 49330 [details] proposed path including prefs UI
Eugene, the patch seems to be incomplete (looks like everything from org.eclipse.ui.workbench.texteditor is missing).
Created attachment 49394 [details] proposed path with prefs UI
I looked at the patch and here my general comment. Please don't be scared of the length ;-) It would help if the extension point schema would have some minimal doc. From what I can interpret the extension point allows to contribute detectors to the whole IDE and it is not clear whether they will be used in an editor a view a dialog or whatever. This is not acceptable since this extension point is in the editor land and hence a detector needs to be contributed to an editor ID or editor class/interface . Contributions for the targetEditorClass would be for the whole inheritance tree while contributions to the targetEditorId would be for said editor only. See the new rulerColumns extension point for a template. The detector should be configured with an editor. You can then get the path (or other attributes) from that editor. Rename the AbstractHyperlinkDetector to AbstractEditorHyperlinkDetector to reflect that it is editor centric. Get rid of the HyperlinkDetectorService. Instead make the registry API with a public static accessor method. The preference UI (as marked by yourself with TODOs) is still in works but goes into the right direction. We should also show that contribution target in the UI, e.g. 'Text Editor'. This might be a little harder for contributions to a targetEditorClass. For that case it might be a good idea to also have a targetDescription attribute in the schema. Other minor comments in no particular order: - not only public but also internal packages need to be declared in MANIFEST.MF - new API packages need a package.html with its description - don't use this. qualification for fields - please read the http://dev.eclipse.org/conventions.html and follow the additional rules used in the Platform Text project: * instance variables start with an f prefix * non final static variables start with an fg prefix * we use the compact assignment form (a= b, i.e. no space on the left of equals)
(In reply to comment #35) > Other minor comments in no particular order: > - not only public but also internal packages need to be declared in MANIFEST.MF > - new API packages need a package.html with its description > - don't use this. qualification for fields > - please read the http://dev.eclipse.org/conventions.html > and follow the additional rules used in the Platform Text project: > * instance variables start with an f prefix > * non final static variables start with an fg prefix > * we use the compact assignment form (a= b, i.e. no space on the left of > equals) Daniel, it is really hard to keep up with your requirements when manually formatting code. Most of these comments would be addressed if you had formatting and other code style settings committed per projects in CVS like other Eclipse projects did (see Team/CVS for example). This will help not only me, but all other contributors.
Daniel, once again, please note that this issue and my patch adresses hyperlink detectors not only for editors, but virtually any decorated text components. See comments from Ed Burnette and Brock Janiczak. So, it does seem weird to have this extension point and configuration UI under editors, but I can live with that as long as it can handle not only editors. If you feel that Platform Text is inapropriate component for such goals please reassign this issue to the more appropriate component.
re comment 36: I agree, two of these four would be addressed. >If you feel that Platform Text is inappropriate component for such goals please >reassign this issue to the more appropriate component. Yes, this bug is really about contributing to (textual) editors (see initial comment 0 and Mik's patch targeted against the Java editor). Sorry if I didn't make that clear. I suggest you submit a new bug against Platform UI requesting to provide a general mechanism to contribute detectors globally i.e. without any target. The problem I see with this approach is that it leaves users without a clue what wizard, dialog, view or editor actually uses/supports it.
(In reply to comment #38) > >If you feel that Platform Text is inappropriate component for such goals please > >reassign this issue to the more appropriate component. > Yes, this bug is really about contributing to (textual) editors (see initial > comment 0 and Mik's patch targeted against the Java editor). Actually comment 0 does not say anything about editors. > Sorry if I didn't > make that clear. I suggest you submit a new bug against Platform UI requesting > to provide a general mechanism to contribute detectors globally i.e. without > any target. The problem I see with this approach is that it leaves users > without a clue what wizard, dialog, view or editor actually uses/supports it. Aren't hyperlinks decorated in the styled text? I still don't understand why are you showing so much resistance to the proposed patch...
(In reply to comment #38) > >If you feel that Platform Text is inappropriate component for such goals please > >reassign this issue to the more appropriate component. > Yes, this bug is really about contributing to (textual) editors (see initial > comment 0 and Mik's patch targeted against the Java editor). Actually comment 0 does not say anything about editors. > Sorry if I didn't > make that clear. I suggest you submit a new bug against Platform UI requesting > to provide a general mechanism to contribute detectors globally i.e. without > any target. The problem I see with this approach is that it leaves users > without a clue what wizard, dialog, view or editor actually uses/supports it. Aren't hyperlinks decorated in the styled text? I still don't understand why are you showing so much resistance to the proposed patch... Bug 156219 is created to discuss the same issue for Platform UI component. https://bugs.eclipse.org/bugs/show_bug.cgi?id=156219
>Actually comment 0 does not say anything about editors. Mmh, are we talking about the same comment in the same bug? I read: "There should be a way to add own IHyperlinkDetectors to text editors" >I still don't understand why are you showing so much resistance to the proposed >patch... Because it does not fit into the Platform Text (and in my opinion also Eclipse SDK) architecture: extensions are targeted and not contributed globally. Your scenario requires an IPath that's why it is now configured with it in you path. But what if another client needs access to e.g. an IContentType or an ITextEditor? IPath just fits your case but it is by far something that can globally fit.
(In reply to comment #41) > >I still don't understand why are you showing so much resistance to the proposed > >patch... > Because it does not fit into the Platform Text (and in my opinion also Eclipse > SDK) > architecture: extensions are targeted and not contributed globally. Your > scenario requires an IPath that's why it is now configured with it in you path. > But what if another client needs access to e.g. an IContentType or an > ITextEditor? IPath just fits your case but it is by far something that can > globally fit. Hmm. For some reason I though that detectHyperlinks() method gets editor instance as a parameter. IHyperlink[] detectHyperlinks(ITextViewer textViewer, IRegion region, boolean canShowMultipleHyperlinks); apparently textViewer isn't sufficient there. Speaking of global services provided by platfor, I can say that some of them are actually used without specific target. For instance spell checker is recently hooked up to CVS commit dialog and all over Mylar's UI. So, it would be really beneficial for the platform if we could have something like that for the hyperlinks.
The difference between the engine and the detectors is that the engine is there and can be used but with your patch all (enabled) global detectors are automatically contributed to all source viewers. This can also result in performance problems.
(In reply to comment #43) > The difference between the engine and the detectors is that the engine is there > and can be used but with your patch all (enabled) global detectors are > automatically contributed to all source viewers. That is the whole point to get it contributed to all source viewers! > This can also result in performance problems. It may as well not result in performance problems. Besides, as you pointed out every detector contributed this way can be easily disabled. Even those you couldn't disable up till now (e.g. url detector).
(In reply to comment #44) > That is the whole point to get it contributed to all source viewers! But as an implementor of a text editor I'm still able to make the decision what IHyperlinkDetectors I want to allow in my editor or not?
(In reply to comment #45) > > That is the whole point to get it contributed to all source viewers! > But as an implementor of a text editor I'm still able to make the decision what > IHyperlinkDetectors I want to allow in my editor or not? I think that in most of the cases you either have detectors fairly specific to your own editor and not reusable (so no point ot put them into the extension point) or you allow any other detectors to analyze your texts. Even those detectors your editor does not know about!
Hi Eugene, after sleeping over it, we might find a solution in the middle i.e. one that does not automatically globally contribute to every text viewer (which I think Platform UI will also turn down) but also not just limited to editors. But let me start with a minor reminder regarding TextSourceViewerConfiguration: I know it is not used as such and the name is misleading but the TextSourceViewerConfiguration's Javadoc clearly states to be the viewer configuration for a text editor (yes for a text editor) and as such it also accesses and uses text editor preferences. Now let's look at a possible solution: The extension-point would look something like this: - target [class: must inherit from IAdaptable] - allowSubclass [default: false] - adaptable [default: false] - targetDescription Now, since TextSourceViewerConfiguration is designed to be for text editors we could add a new constructor that takes the editor as argument and do something like this: IHyperlinkDetect[] getHyperlinkDetectors(...) { ... HyperlinkDetectorRegistry.getDefault().createHyperlinkDetectors(fTextEditor); ... } But since I guess that already many people use the TextSourceViewerConfiguration in other non-editor related scenarios we could do a little better: We add a new IHyperlinkDetectorTarget with getTarget() that returns an IAdaptable. The TextSourceViewerConfiguration's getHyperlinkDetectors(...) would do something like this: adapter= sourceViewer.getAdapter(IHyperlinkDetectorTarget); if (adapter != null) { target= adapter.getTarget(); HyperlinkDetectorRegistry.getDefault().createHyperlinkDetectors(target); } The registry will then collect all detectors from the contributed extensions that map the target. Configurations that inherit directly from SourceViewerConfiguration could either copy the above code or even simpler pass in the target directly i.e. no need to use/define an IHyperlinkDetector. ==> the above solution would not change anything in any viewer or editor unless the viewer supports an IHyperlinkDetectorTarget adapter or the configuration explicitly collects the detectors with the target of its choice. It is up to the viewer and/or viewer configuration to decide whether the target is an IPath, an ITextEditor or whatever. This means that the creator decides whether to support the extension point and what the target is. How does that sound?
I'd like it to work in Console views (program output, Ant output, CVS output, etc.), would #47 allow that? How about the text field of an error log detail dialog (I'm thinking tracebacks). That would be nice but not as important (to me) as Console views.
>I'd like it to work in Console views (program output, Ant output, CVS output, >etc.), would #47 allow that? Not out of the box i.e. not without the console defining what target it. But it would also not work out of the box with Eugene's patch because as far as I can see most console viewers don't even use a configuration. >How about the text field of an error log detail dialog (I'm thinking >tracebacks). That would be nice but not as important (to me) as Console views. It would work as soon as the dialog's text area is a source viewer and registers a target. All hyperlink detectors registered for that target would then work. If the mechanism should work in dialogs and wizards then a JFace only (i.e. not depending on text viewer and IDocument) solution is the better match (see bug 156219).
Regarding the proposal in comment #47, does this basically take away from what I wanted done automatically in comment #5? Meaning, is the HyperlinkDetectorTarget just the editor and then each hyperlink detector associated with the editor has to determine what editor, content type, partition type it is in?
No, because the Java detector currently expects the Java editor and accesses attributes from it. Unless this detector is rewritten and takes e.g. a content type or site or whatever it won't work (unless of course the JSP editor can provide a JavaEditor subclass instance). However, once the Java detector is converted and contributed to a generic target and if the JSP editor specifies such a target then yes, they would map and it *should* work. Side note: IHyperlinkDetectorTarget.getTarget() probably needs to return IAdaptable[].
Daniel, are you going to take over this patch and implement stuff suggested in comment #47? I suppose you meant 3.3M2 target and not 3.2M2?
>Daniel, are you going to take over this patch and implement stuff suggested in >comment #47? No, I'm still waiting for comments, especially you did not yet answer whether this would fit your needs or whether you gave up in favor of bug 156219. I'm also waiting for some comments in bug 156219 to see the direction of Platform UI. If you think my proposal fits your needs then I'd be glad if you could continue to provide a patch of quality that can be committed to head. >I suppose you meant 3.3M2 target and not 3.2M2? Sorry, didn't want to touch the '3.3' target.
(In reply to comment #53) > If you think my proposal fits your needs then I'd be glad if you could continue > to provide a patch of quality that can be committed to head. The thing is that it does not help much to immediately resolve issues that motivated me to work on this. Basically it will require changes in many other places before I could do something useful with this extension point. However I am still willing to invest few hours of my time to bring patch to some functional state where you can take over. So, you'd have to tell what exactly what need to be done in the current patch to get us there. Also, I can't spend any time on manual code formatting and stuff like that.
OK I see. I suggest we first wait how bug 156219 evolves. Maybe this obsoletes this bug and Platform Text can make use of it. However, I'll make sure that we have something for 3.3.
We should use a regex patterns in extension point to prevent early loading (see also comments in bug 156219).
Daniel, generally regexp patterns (and static regexp patterns) are too limited and far away from the really practical use. As I've been trying to point out earlier, some of the patters can be only built after reading specific metadata related to particular resource or other artifact (entry in history view, commit in the change set, etc). Such as, its link to issue tracking system, which then allow to construct a template or regexp for this resource (so, template for other resource, ie from a different project, won't work).
I didn't say "only allow regex patterns" ;-) There are many URL detectors that will be able to live with just regex (and knowing it's e.g. the Java editor or Java content type).
Here's a new proposal for this that I plan to put into M5. I will try to attach a first cut of a patch tomorrow. Let me know what you think. 1. new ITextViewerExtension7 will allow to return hyperlink detector target ids and it will be possible to register and unregister such target ids via code. This will allow to contribute detectors for a part that has more than one text viewer. 2. a "hyperlinkTargets" extension point will allow define, label and describe a target. This makes it easier for contributors: they don't need to parse the code. See open issues below regarding the label for those targets. 3. I will contribute target ids for the source and the Java source viewer and other viewer owners (e.g. Ant) will be asked to do the same. 4. a "hyperlinkDetectors" extension point will allow to contribute an IHyperlinkDetector to a registered target. There will be no restrictions by regex patterns because it turned out that this isn't sufficient for most clients. However, a flag will allow to tell whether the extension should only be available if the contributed plug-in is loaded The above will allow clients to register a hyperlink detector to any source viewer (Eugene, this should address your scenario) and it will also allow other viewers to fetch contributions by registering the corresponding target id, e.g. JSP viewer could register the JDT source viewer target to grab its detectors. Note however, that those detectors might not work in another context because for example no Java model information can be found. I might also add optional (partitioning, partition) to the hyperlinkDetector extension point to allow finer grained contributions. Open Issues: - context: how do we provide context (e.g. the resource that's behind a viewer) to the detectors? - preference UI: how can we explain the (source) viewer to the user i.e. how do we label such targets that don't correspond to an editor?
>- preference UI: how can we explain the (source) viewer to the user i.e. how > do we label such targets that don't correspond to an editor? I guess in a first cut I'll put the preference page under 'Text Editors' since other settings that are used in textual viewers (i.e. not editors) are also taken from there.
Daniel: this is sounding very promising. To make sure that I understand, could you outline a scenario for an example "bug 123" hyperlink detector? Say we would want to do something like define it working for the following targets: * Java comments (both Java doc and regular) * Mylar's Task Editor, e.g. used for viewing and posting Bugzilla comments (we'd make our own custom hyperlinkTarget) * History comment viewer (not sure if we could define and add such a target or Platform/Team has to) The really neat thing about your proposal is the reuse aspect. It sounds like we may be able to somehow tell our Task Editor to use the "java stack trace" detector's target (used by Console), and in this way get the identical stack hyperlinking. This particular case is minor for Mylar because we probably need to keep our custom one that handles bad formatting, but this kind of reuse of hyperlink detectors could be very good in general because it would result in predictability and consistency for users. In terms of UI, it seems like the key thing is that the user be able to turn off any contributed hyperlink detector. And maybe users don't need to know about the individual source viewers, which are a sort of implementation detail, as long as the preference page shows which detector associated with which targets (and targets have clear labels)? This seems similar to the platform Label Decorators approach: the user can't specify which view will get which decorators, and from that we get a nice consistency. But if some decorator is bugging them, they can easily turn it off.
(In reply to comment #59) Does "target id" mean any arbitrary text string? Maybe we can add something to register custom context provider to the source viewer. Then context can be retrieved from such provider and passed to the hyperlink detector when it is invoked for detecting hyperlinks.
Created attachment 57498 [details] Proposed Fix (work in progress) This patch contains the proposed fix and the existing hyperlink detectors converted to extensions. It also provides a solution to pass the context. What's missing in the patch is the preference UI, tests and integrity checks (e.g. whether there's a contributed target for a given targetId).
re comment 62: see my patch which contains a solution for the context. Basically the target specifies what it supports hence when you register your detector you know what you can access. re comment 61: >Say we >would want to do something like define it working for the following targets: >* Java comments (both Java doc and regular) You would target "org.eclipse.jdt.ui.javaCode" and in your hyperlink detector you do the work i.e. decide in which partitions it is available. * Mylar's Task Editor, e.g. used for viewing and posting Bugzilla comments (we'd make our own custom hyperlinkTarget) You register a target and a detector via plugin.xml and your source viewer configuration would provide the target along with the context. * History comment viewer (not sure if we could define and add such a target or Platform/Team has to) Platform/Team would have to define a target via plugin.xml and specify what context it provides. You can then contribute a detector to that target. Subclasses of TextSourceViewerConfiguration.getHyperlinkDetectorTargets(...) would normally include the inherited ones (to simplify that I declared the return value as a Map) and hence if you register your hyperlink for the "org.eclipse.ui.DefaultTextEditor" target it will appear in all editors/source viewers that subclass that config. That's pretty close to the "global" contribution that Eugene had in mind.
Would these new features address bug 164240 (I guess so), but more challenging bug 164241 (colorizing/featurizing "hit text")?
Bug 164240 is something that the contributor needs to implement and bug 164241 is beyond the scope of this PR which only deals with on-demand hyperlinks. Additional note to my patch: it currently loads the plug-in when the hyperlink detectors are installed on the viewer - this will be changed for the final version.
Re comment 66, I was just wondering whether it would be possible to maybe include such a feature ("colorizing/featurizing" of the hit text) when you're already doing the actual text-matching and "hooking" features: They would both alter the text - one is about adding a link to it, the other is about setting color to it - I kind of hoped that it was possible to kill to birds with one stone!
Created attachment 57517 [details] proposed fix This revised patch does lazy loading, integrity checking and error handling. The only missing piece is the preference UI.
This is going to be so cool. Daniel, I have a question similar to comment#67 because the concern is so related, but I understand if it is out of scope. Once we have these hyperlinks, I believe that most integrators will want to highlight them (e.g. blue color, underline) so that they are discoverable. Otherwise we rely on documentation or accidental discovery via mousing over. Right now I assume that would involve the usual custom RuleBasedScanner and damage/repair code. Since the detection mechanism and the detection of what should be highlighted will use the same mechanism, is it possible to have a mode or preference (can be off by default), where the viewer can be permanently highlights on-demand hyperlinks for specific partitions? I think that the following could work really well as a default SDK configuration, and would help users get the most out of this feature. * The Java editor editor highlights all hyperlinks in the comment partition with as a subtle blue dashed underline. Mousing over that underline with Ctrl depressed reatains the current behavior: link goes blue and is clickable. * Other partitions that show hyperlinks mixed in with unstructured text have the same behavior, e.g. Mylar Task Editor. The idea is that partitions that contain mostly structured info (e.g. Java, XML, HTML) do not highlight eagerly, while partitions with mostly natural language info and some structured elements (e.g. "bug 123" or Java refs in comments, other text editors) do highlight. This might be something that can be determined globally by having those two partition types: structured text and freeform text. I think that the mental model is simple for users to understand, and hyperlink overload for such partitions is very unlikely (natural language documents tend to be dominated by unstructured text, as we can see from most the ratio of links on typical HTML pages).
This may be too complex for the general hyperlink detector extension point, and I'm not sure if this is even using them right, but could the hyperlink detector extension point utilize expressions/property testers? This would help in the partitioning/partitions case where you want to be a little more specific about where your hyperlink detector can work. Some sort of additional property that the target can check before actually instantiating the detector. Regarding comment #61 > In terms of UI, it seems like the key thing is that the user be able to turn > off any contributed hyperlink detector. And maybe users don't need to know > about the individual source viewers, which are a sort of implementation > detail, as long as the preference page shows which detector associated with > which targets (and targets have clear labels)? Even if most users might not be interested in the hyperlink targets, it would be nice to offer some sort of filter in case the list of detectors get long. Perhaps at the top of the page is a "Show detectors for" combobox with all the targets and also the option of "All" Then show all the appropriate detectors for the target. For each detector, give it a checkbox for enablement and also list all the other targets this detector applies to (in case the user doesn't realize that turning off java hyperlinking in the jsp target will also turn it off in the java code target)
Mik, I fully understand that you'd like to get all in one ;-) However, the on demand links and the visible hyperlinks not only differ from the LAF but also how they work internally: the on demand links are tuned to detect a link at a given region and (in the future) the UI might be able to handle/show more than one possible contributed link for a given region. On the other hand the visible hyperlinks will probably be regex based pattern rules that must be optimized for syntax coloring in order to not degrade overall syntax coloring and typing performance. Another problem I see when providing visible hyperlinks for some of the links but not all is that even more clients won't know about the on demand links. Since the text framework currently doesn't use the latter approach itself I see it as a too big risk to start at this point. We might want to investigate pluggable syntax coloring (see bug 42550) closer during the next release and if so, we could extend the hyperlinkDetecors extension point at that time. Amy, I thought of property testers too but couldn't find much properties that I would really use, the only one that really came to mind was the document partition. So, if you can/would need this I'd be happy to add that as optional attributes to the extension point: if the 'partitioning' attribute doesn't match getConfiguredDocumentPartitioning(...) then I would not even install the detector and later I'd check the 'partition' before creating and calling the detector. Regarding the preference UI: >"and also list all the other targets this detector applies" This is not feasible because I have no clue - who acts as a target - whether someone extends that unknown target For example a view could show two different viewers which it registers as different targets. It might implement one by extending the targets from the TSVC and the other by replacing (i.e. ignoring) the inherited targets. This could be solved by declaring the inheritance chain in the hyperlinkDetectorTargets extension point but I thought that it is more natural to do this in the code. In addition, currently all textual features that can be configured via 'Text Editors' pref pages cannot be individually adjusted for subclasses. This would be the first one to go down that path. Having said that I plan to surface the target on the preference page i.e. not only the hyperlink name will appear there.
Fixed in HEAD, including preference UI Available in builds > N20070126-0010. Let me know what you think. For now I did not surface the description in the preference UI yet and I'm not sure whether we really need the detector and the target description. I'll either show it in the UI or remove it for the final release. Please use bug 171817 for this discussion.
Daniel: in order to provide feedback I tried to move Mylar to I20070130-0800, but PDE was throwing constant errors that prevent projects from building, and things like Show View -> Other didn't work. I'll assume that these are known bugs and we'll try to move again as soon as an I-build is available next week.
Mik, the Show View bug has been reported on the list but we are not aware of the PDE problem. Please report a bug. Note that we will do a I-rebuild today at 0800.
(In reply to comment #74) > Mik, the Show View bug has been reported on the list but we are not aware of the > PDE problem. Please report a bug. Reported bug 172975. The work-around for me was to remove the "configuration" directory of the Target Platform.
Verified that the functionality is in. Will be tested during test pass.
Verified that the functionality is extensible, and the preference UI is slick. This is a *really* nice addition to the Platfrom. Tomorrow's Mylar 2.0M1 will have the "bug 123" and related hyperlinking working in the Java editor, which is fantastic. Minor comments: 1) To get the IResource we manually get the active editor from the active page, which is not ideal (there may be a better way). 2) All Java detectors works for all text in the editor, so "bug 123" will be linked even if it is code, whereas it might be good to limit to String or comment partitions (so will URLs). Practically speaking I don't currently currently foresee a problem with this, but at some point detectors might want this finer grained control.
1) It depends on the hyperlink detector target. The Java source viewer's target for example offers an ITextEditor adapter i.e. if your hyperlink detector extends AbstractHyperlinkDetector you can get the editor like this: getAdpater(ITextEditor.class) 2) >but at some point detectors might want >this finer grained control. What finer control do you have in mind? What more than the whole document can we give? The problem here is rather that JDT uses the DEFAULT partition type for code and hence you cannot easily detect on which partition your detector is active.
1) Thanks. 2) I don't have a strong use case other than what was mentioned in comment#77, so the current level of control is sufficient. Tagging as "greatbug" because this really was an amazing one due to all of the challenges and input that it involved. At EclipseCon I found myself telling the story of this bug several times, and wanted to give a big thanks to Platform/Text for putting this functionality into our hands for 3.3. The ability to link from an SDK editor to the structured elements that your extension provides makes for a great showcase of Eclipse's ubiquitous extensibility.
