Community
Participate
Working Groups
In Eclipse IDE, I see many message dialogs giving instruction about opening a wizard or the preference page. Giving such instructions to user is nice, but in many cases, it would be even better to allow user to perform such operations in one click. For example a message dialog with "configure proxy" would have a link to immediately open the proxy settings page. Those message dialog should also allow selectionListeners for those links. If MessageDialog provide links by default, it will encourage consumers to take advantage of them, and to be more helpful to their users.
I believe the most appropriate implementation is to create a new "MessageDialogWithLink" class, that would basically be a clone of the MessageDialog, but with all methods having a new parameter for selection listeners on link. WDYT?
New Gerrit change created: https://git.eclipse.org/r/62785
Seems like a great idea. What would be handy is if we could support handling http*:// links out-of-the-box, and even craft a URL style to execute Eclipse Commands (e.g., to launch a wizard). Perhaps we could support the Eclipse Welcome/Intro style URLs (e.g, "http://org.eclipse.ui.intro/execute?command=org.eclipse.ui.file.import%28importWizardId=org.eclipse.ui.wizards.import.ExternalProject%29"). Do you think we can just replace the Label with Link always, or perhaps check if the text contains an <a> marker?
(In reply to Brian de Alwis from comment #3) > Seems like a great idea. What would be handy is if we could support > handling http*:// links out-of-the-box, and even craft a URL style to > execute Eclipse Commands (e.g., to launch a wizard). Perhaps we could > support the Eclipse Welcome/Intro style URLs (e.g, > "http://org.eclipse.ui.intro/execute?command=org.eclipse.ui.file. > import%28importWizardId=org.eclipse.ui.wizards.import.ExternalProject%29"). I didn't think about that. So it would be mostly a matter automatically adding a specific selection listener on Links? IMO it's a good idea, but I would consider as a second iteration. > Do you think we can just replace the Label with Link always, or perhaps > check if the text contains an <a> marker? In the proposed Gerrit patch, I added optional SelectionListeners to openXXX(...) methods. Then the strategy is to show a link when some selectionListeners are provided, and to keep a label when there are none.
(In reply to Mickael Istria from comment #4) > In the proposed Gerrit patch, I added optional SelectionListeners to > openXXX(...) methods. Then the strategy is to show a link when some > selectionListeners are provided, and to keep a label when there are none. Yup, I saw that and hence my suggestion, as it's a bit of a pain to *require* users to define a selection listener for what will be (as you point out in comment 0) a frequent case. We could have a default SelectionListener on JFace's Policy class that just directs to SWT's Program#launch(), and then have the Workbench configure it with one that knows more (JFaceUtil#initializeJFace()).
In most of the case I've seen (p2 update, EGit messages...) there is no http link or commands to use in the MessageDialog, and the MessageDialog would really require a specific SelectionListener to perform the right action. So I'm not sure enforcing a default listener is actually helpful. Do you have examples of MessageDialog that would really benefit from such change?
My thoughts: we only do this if we find an "<a[^>]*>" in the label string. We make it as easy as possible: having to roll your own SelectionListener for common cases is annoying.
(In reply to Brian de Alwis from comment #7) > My thoughts: we only do this if we find an "<a[^>]*>" in the label string. > We make it as easy as possible: having to roll your own SelectionListener > for common cases is annoying. I'm not against the proposed change, but I'm unsure whether HTTP links or commands are actually a common case. So far, I've seen none in the (few) MessageDialog that would take advantage of links.
There are a few StackOverflow questions around this very topic: http://stackoverflow.com/questions/3968620/how-can-i-add-a-hyperlink-to-a-jface-dialog http://stackoverflow.com/questions/29374160/add-link-to-messagedialog-message In my own projects, I created a separate openError-like dialog to use a link to allow: * referencing online documentation * mailto:support@ messages Some of my client's Swing projects made use of Swing's abilities to render HTML to include links. IIRC it required adding a selection listener too to do the call to Desktop#browse() (and of course, some places didn't do it correctly). My point is really: if we expose being able to embed hyperlinks, developers will make use of it, and so let's help them do it the right way. But it can be retrofitted later. One positive aspect with your approach, to only use Link if a SelectionListener is provided, is that there's no chance that existing applications will ever use a Link, in case they have an "<a>" in their dialog text.
Maybe we can simply include into Platform UI some basic listener for links, such as OpenHTTPWebpageSelectionHandler, MailToSelectionHandler, CommandExecutionSelectionHandler and let users consume them when it makes sense. The JavaDoc could mention those utility handler to promote them to consumers.
(In reply to Brian de Alwis from comment #9) > One positive aspect with your approach, to only use Link if a > SelectionListener is provided, is that there's no chance that existing > applications will ever use a Link, in case they have an "<a>" in their > dialog text. I didn't think about it but indeed, the current approach doesn't change behavior of already existing dialogs at all (it even keeps the Label instead of using a Link when no listener is configured). Do you think the current patch is good enough to be merged? And then we could consider creation of new handlers and automatic addition of some of them later?
(In reply to Mickael Istria from comment #11) > Do you think the current patch is good enough to be merged? And then we > could consider creation of new handlers and automatic addition of some of > them later? I feel like I'm bike shedding a bit, but this API will last a long time. But there are two points that cause me to hesitate: 1. The use of SelectionListener… ellipsis seems as convenience to avoid an explosion in methods. I wonder if we aren't better to use this as an opportunity to move to a builder-style creator. 2. Is exposing SelectionListener the right interface? If chaining multiple listeners, the implementors need to remember set "event.type = SWT.None" to prevent the event from propagating down the chain. That seems like something that should be handled by the class; we could use a Java8 Function<String,Boolean>?
@Brian: Would you like to take ownership of this bug? Despite I'm interested in it, it's not high enough in my priority queue to guarantee I'll be able to make progress on that in a good timing. So if you'd like to continue that work, that'd be nice (especially since you have pretty good ideas about it). Practically, feel free to amend my commit, or just copy-paste what you need in it for another change; my code is yours ;)
Mass move to M6
Gerrit change https://git.eclipse.org/r/62785 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d5de69d04b4cf51061fab446cdb0ae40aada17d9
Mickael, please add N&N entry for this one.
We should change some dialogs to use this.
(In reply to Dani Megert from comment #17) > We should change some dialogs to use this. I have it in my queue to change one in p2.
New Gerrit change created: https://git.eclipse.org/r/125103
Gerrit change https://git.eclipse.org/r/125103 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=607e1ee5937c7d1be8941e737ff36815c4208f2e
(In reply to Eclipse Genie from comment #20) > Gerrit change https://git.eclipse.org/r/125103 was merged to [master]. > Commit: > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=607e1ee5937c7d1be8941e737ff36815c4208f2e > That reverts the change as it broke API. You can't change the signature of API methods!
(In reply to Dani Megert from comment #21) > (In reply to Eclipse Genie from comment #20) > > Gerrit change https://git.eclipse.org/r/125103 was merged to [master]. > > Commit: > > http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=607e1ee5937c7d1be8941e737ff36815c4208f2e > > > > That reverts the change as it broke API. You can't change the signature of > API methods! It would be more helpful if you put the exact reason in these messages :).
The change adds some varargs parameters to the API methods. So the client code doens't need to change with the addition as no argument works for varargs. Aren't varargs also bytecode compatible against version of the same method without varargs?
(In reply to Mickael Istria from comment #23) > The change adds some varargs parameters to the API methods. So the client > code doens't need to change with the addition as no argument works for > varargs. > Aren't varargs also bytecode compatible against version of the same method > without varargs? It's a bit more complicated. Eclipse (and javac) allow to compile against 1.3 and 1.4. Clients who have projects with that setup won't compile anymore as varargs aren't know there. Also, I'd like to repeat what I've said many times: please setup API Tools. That would have told you that you broke the API. Description Resource Path Location Type Missing @since tag on addLinkSelectionListener(SelectionListener) IconAndMessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 88 @since tag problem Missing @since tag on linkSelectionListeners IconAndMessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 70 @since tag problem Missing @since tag on messageLink IconAndMessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 60 @since tag problem Missing @since tag on openConfirm(Shell, String, String, SelectionListener[]) MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 547 @since tag problem Missing @since tag on openError(Shell, String, String, SelectionListener[]) MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 560 @since tag problem Missing @since tag on openInformation(Shell, String, String, SelectionListener[]) MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 573 @since tag problem Missing @since tag on openQuestion(Shell, String, String, SelectionListener[]) MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 588 @since tag problem Missing @since tag on openWarning(Shell, String, String, SelectionListener[]) MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 601 @since tag problem The field org.eclipse.jface.dialogs.IconAndMessageDialog.linkSelectionListeners has been added to a class IconAndMessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 70 Compatibility Problem The field org.eclipse.jface.dialogs.IconAndMessageDialog.messageLink has been added to a class IconAndMessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 60 Compatibility Problem The major version should be incremented in version 3.15.0, since API breakage occurred since version 3.14.0 MANIFEST.MF /org.eclipse.jface/META-INF line 5 Version Numbering Problem The method org.eclipse.jface.dialogs.MessageDialog.openConfirm(Shell, String, String) has been removed MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 39 Compatibility Problem The method org.eclipse.jface.dialogs.MessageDialog.openError(Shell, String, String) has been removed MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 39 Compatibility Problem The method org.eclipse.jface.dialogs.MessageDialog.openInformation(Shell, String, String) has been removed MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 39 Compatibility Problem The method org.eclipse.jface.dialogs.MessageDialog.openQuestion(Shell, String, String) has been removed MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 39 Compatibility Problem The method org.eclipse.jface.dialogs.MessageDialog.openWarning(Shell, String, String) has been removed MessageDialog.java /org.eclipse.jface/src/org/eclipse/jface/dialogs line 39 Compatibility Problem
(In reply to Mickael Istria from comment #23) > The change adds some varargs parameters to the API methods. So the client > code doens't need to change with the addition as no argument works for > varargs. > Aren't varargs also bytecode compatible against version of the same method > without varargs? I didn't read into the full detail of the spec but it looks like it's not binary compatible. It only works if the client code is recompiled. I did a quick check where I kept the old client class files and they failed to run after adding varargs.
(In reply to Dani Megert from comment #25) > (In reply to Mickael Istria from comment #23) > > The change adds some varargs parameters to the API methods. So the client > > code doens't need to change with the addition as no argument works for > > varargs. > > Aren't varargs also bytecode compatible against version of the same method > > without varargs? > > I didn't read into the full detail of the spec but it looks like it's not > binary compatible. It only works if the client code is recompiled. I did a > quick check where I kept the old client class files and they failed to run > after adding varargs. Changing array to varargs is binary compatible, see https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_classes_-_API_methods_and_constructors
(In reply to Lars Vogel from comment #26) > Changing array to varargs is binary compatible, see > https://wiki.eclipse.org/Evolving_Java-based_APIs_2#Evolving_API_classes_- > _API_methods_and_constructors HEre it's a bit different, we change nothing (no arg) to a varargs.
New Gerrit change created: https://git.eclipse.org/r/137976
