Community
Participate
Working Groups
Build ID: M20060921-0945 Steps To Reproduce: 1. Goto Bugzilla 2. Open User Preferences 3. Goto General Preferences 4. Change comment ordering to "Newest to Oldest" 5. Run Eclipse with Mylar 6. Open any bug in the Mylar bug editor using the same user credentials as used with preference changes. 7. The Description field will contain the text of the last comment entered (chronologically) instead of the first. More information: I'm running against a Bugzilla 2.22.1 server.
That is a cool feature. I didn't know you can do that. :-)
I don't think that setting is exposed in the config.cgi so we wouldn't be able to adapt to this preference change (unless we add capability to parse user's preferences). Perhaps in the short term we could add an advanced configuration section to the bugzilla repository configuration that would included options such as this (although I cringe at the thought of adding anything else to the configuration page).
(In reply to comment #2) > I don't think that setting is exposed in the config.cgi so we wouldn't be able > to adapt to this preference change (unless we add capability to parse user's > preferences). Perhaps in the short term we could add an advanced configuration > section to the bugzilla repository configuration that would included options > such as this (although I cringe at the thought of adding anything else to the > configuration page). > Adding this to the repository configuration may be ugly, but it seems necessary as this is a feature that some installations may use.
Rob, can we just sort entries by date?
Adding sort capability to the editor would make sense. The only issue being that bugzilla sends the description as comment #0 so unfortunately we would still need to handle this in some fashion.
I meant that you do have date for each comment. Obviously description will be the earliest comment according to its date and not the position in the result returned by bugzilla.
Sorry, still waking up from vacation. :) You're right Eugene, we do have the date stamp for all the comments/description so this should be straight forward.
(In reply to comment #5) > Adding sort capability to the editor would make sense. The only issue being > that bugzilla sends the description as comment #0 so unfortunately we would > still need to handle this in some fashion. > Note, this invalid assumption is the core issue, Bugzilla does NOT always send the description as comment #0. Comment #0 is only the description if ordering is 'Oldest to Newest'. I apologize if I'm just restating the obvious, but I always like to make sure everything is really clear.
In terms of UI for toggling the sort order on the editor: I suggest using the same button location as the plugin.xml editor's All Extensions section on the Extensions page. But I think that the full-sized icon is probably bettern than the shrunken one.
*** Bug 190011 has been marked as a duplicate of this bug. ***
*** Bug 198861 has been marked as a duplicate of this bug. ***
Is there a way to have access to the Bugzilla preferences so that here is a way to detect if comment 0 or comment n is the description? I think it is the correct way to show comments in mylyn in the defined order of bugzilla pereference. A sort button is nice but I think a change of the bugzilla perference is for me the better (clearer) way.
-1 for using bugzilla preferences for this. Like it been said in comment #6, each comment has timestamp and description is just a comment with the earliest timestamp. Also see/vote for bug 195656 that is requesting an option to show comments in reversed order in the task editor.
Related to bug 195656. If we consider this we need to consider how this affects other parts of the layout (e.g. Actions section needs to be near New Comment, which needs to be near the last comment).
(In reply to comment #14) > e.g. Actions section needs to be near New Comment, which needs to be near the last comment). I can't find bug report for that, but there should be one about making editor actions vs. editor save story. It could allow to eliminate need in the action section, or move it out of the editor layout, i.e. in a dialog/wizard.
Created attachment 84246 [details] ImageHyperlink to sort Comments Here is an ImageHyperlink that can sort the Comments. What do you think about this. Next I will attache the two new gif files
Created attachment 84247 [details] mylyn/context/zip
Created attachment 84248 [details] sort-down.gif put the file to /org.eclipse.mylyn.tasks.ui/icons/etool16/sort-down.gif
Created attachment 84249 [details] sort-up.gif put the file to org.eclipse.mylyn.tasks.ui/icons/etool16/sort-up.gif
Created attachment 84263 [details] compleate patch the patch from comment#16 did only add an action to the comment section. This patch show the comments and the description in the right order. Bugzilla knows the following values for "When viewing a bug, show comments in this order" - Site Default (Oldest to Newest) - Oldest to Newest - Newest to Oldest - Newest to Oldest, but keep Description at the top Before this patch the Description was only correct if the Description is the first in the list of Longdescription. This was not true for the option Newest to Oldest. BTW feel free to add a more nicely gif for the sortup and sortdown from comment 18 and 19.
Created attachment 84264 [details] mylyn/context/zip
This is very cool Frank. A few nits: * Task Comment number ideally shouldn't mutable. Is there any way of achieving the same result without adding this new api? * Sort order button should be disabled when Comments section collapsed (for that matter so should the collapse all button) * sortComments() assumes sortSection exists but may not due to createCommentLayout() being overridden * We may also want to make this feature optional (perhaps using a commentsSortable boolean set in implementing editor's constructor.. ideas?) * sortComments has to got to great lengths to get the proper composites to move. Ideally this would be factored into Steffen's editor revamp And as always, the more unit tests we see with patches the easier to verify. In the very least we could test that comments in a retrieved bug report are indeed in the correct order considering the changes made to comment construction in SaxMultiBugReportContentHandler.
Steffen, adding you to the cc here since it would be great if you could include the comment ordering concern in your ongoing decomposition of the Task Editor. See comment#23.
Thanks. From a quick glance at the patch it looks like it should be straight forward to port to the new architecture so I am fine with going ahead here. I have added a note to bug 179254.
(In reply to comment #22) > This is very cool Frank. A few nits: > > * Task Comment number ideally shouldn't mutable. Is there any way of achieving > the same result without adding this new api? If we have no TaskComment.setNumber we must clone all longDescs in SaxMultiBugReportContentHandler.endElement because only the constructor can set the number. > * Sort order button should be disabled when Comments section collapsed (for that > matter so should the collapse all button) Why? Actually you can change the order while all comments are collapsed . If you then expand the show up in the expected order. > * sortComments() assumes sortSection exists but may not due to > createCommentLayout() being overridden Patch for this is comming soon. > * We may also want to make this feature optional (perhaps using a > commentsSortable boolean set in implementing editor's constructor.. ideas?) Patch for this is comming soon. What's about protected boolean supportsCommentSort() used for this? > * sortComments has to got to great lengths to get the proper composites to move. > Ideally this would be factored into Steffen's editor revamp > > And as always, the more unit tests we see with patches the easier to verify. In > the very least we could test that comments in a retrieved bug report are indeed > in the correct order considering the changes made to comment construction in > SaxMultiBugReportContentHandler. What is the rigth way to implement tests? Should I use the http://mylyn.eclipse.org/bugs30 ore can I use an xml file. If xml file where should I store it?
Created attachment 87813 [details] New Patch see comment#25
Created attachment 87814 [details] mylyn/context/zip
(In reply to comment #25) > (In reply to comment #22) > > This is very cool Frank. A few nits: > > > > * Task Comment number ideally shouldn't mutable. Is there any way of > achieving > > the same result without adding this new api? > If we have no TaskComment.setNumber we must clone all longDescs in > SaxMultiBugReportContentHandler.endElement because only the constructor can set > the number. Hmmm yes... this is a problem. Lets go with your current implementation. > > * Sort order button should be disabled when Comments section collapsed (for > that > > matter so should the collapse all button) > Why? Actually you can change the order while all comments are collapsed . If you > then expand the show up in the expected order. The user should see the result of their action. So we should either enable/disable the button, or expand the section upon pressing the direction button if the section was not already opened. > > * We may also want to make this feature optional (perhaps using a > > commentsSortable boolean set in implementing editor's constructor.. ideas?) > Patch for this is comming soon. What's about protected boolean > supportsCommentSort() used for this? That could work. > > And as always, the more unit tests we see with patches the easier to verify. > In > > the very least we could test that comments in a retrieved bug report are > indeed > > in the correct order considering the changes made to comment construction in > > SaxMultiBugReportContentHandler. > What is the rigth way to implement tests? Should I use the > http://mylyn.eclipse.org/bugs30 I'd just create a bug with a few comments and write a test against it.
Patch is looking good Frank. One final point in addition to those in my previous comment. We should store the chosen sort direction in a TasksUiPlugin preference. The preference should likely be connector kind specific as well so you can have two different defaults for Trac and Bugzilla for example. So a preference key along the lines of: "org.eclipse.mylyn.editor.comments.sortDirection."+taskData.getRepositoryKind(); ...would get set. Thoughts?
Created attachment 87910 [details] New Patch With this patch I include the following items. - store sortdirection in preferences (comment#29) - supportsCommentSort() for enable/disable the sort (comment#28) - disable sortbutton if comments are not shown (comment#28)
Created attachment 87911 [details] mylyn/context/zip
Great Frank. Patch applied, and ip log updated. I had to make supportsCommentSort() return false by default and moved creation of the hyperlink around so that we don't get npes when there are no comments on the bug report. Enabled comment sorting on Bugzilla's editor. Steffen, you may want to consider making use of this for Trac and Jira. Mik, the buttons provided for this action look reasonable but you may want to iterate on them.
Cool stuff! I'll hold off on making this available for other connectors until bug 179254 is resolved.
(In reply to comment #30) > - disable sortbutton if comments are not shown (comment#28) The button is disabled but there is no indication that it is not clickable which is a bit odd. I would suggest to either remove it or to grey it out when the section is collapsed.
Created attachment 87968 [details] Followup Patch This pach include the gray out as described in comment 34. Two new gif icon follows as seperated attachments. Rob in comment 28 you wrote: > I'd just create a bug with a few comments and write a test against it. Can you tell me where I can find this test. I want to look at tiis, so that I can write tests by my on in future.
Created attachment 87969 [details] mylyn/context/zip
Created attachment 87970 [details] sort-down-gray.gif put the file to /org.eclipse.mylyn.tasks.ui/icons/etool16/sort-down-gray.gif
Created attachment 87971 [details] sort-up-gray.gif put the file to /org.eclipse.mylyn.tasks.ui/icons/etool16/sort-up-gray.gif
(In reply to comment #32) > Great Frank. Patch applied, and ip log updated. I had to make > supportsCommentSort() return false by default Rob, I wonder what was the rationale for disabling sorting comments? If this operation is there, why not have it always enabled, which would make it consistent across connectors and also reduce the api?
Created attachment 87983 [details] sorting icon Maybe it is better to use icon along the lines with this one? It came from one of the platform plugins.
(In reply to comment #35) > Rob in comment 28 you wrote: > > I'd just create a bug with a few comments and write a test against it. > Can you tell me where I can find this test. I want to look at tiis, so that I > can write tests by my on in future. You could add to a test like BugzillaTaskDataHandlerTest or whatever test class is most appropriate (or create a new one and add it to AllBugzillaTests.java). I've applied your patch Frank and updated the ip log. You will notice I've also switched up the default direction variable so that when the preference is first read the sort direction isn't up by default. (In reply to comment #39) > Rob, I wonder what was the rationale for disabling sorting comments? If this > operation is there, why not have it always enabled, which would make it > consistent across connectors and also reduce the api? We will take this into account when refactoring the editor. I added this as a precautionary measure since if createCommentLayout is overridden and the sort hyperlink never created, with the current code this will result in an eventual NPE. (In reply to comment #40) > Created an attachment (id=87983) > sorting icon > > Maybe it is better to use icon along the lines with this one? It came from one > of the platform plugins. I've committed Frank's icons for now and anticipate Mik will use some platform flook&feel icons as you suggest Eugene.
Created attachment 88084 [details] jUnit Test > > Rob in comment 28 you wrote: > > > I'd just create a bug with a few comments and write a test against it. > > Can you tell me where I can find this test. I want to look at tiis, so that I > > can write tests by my on in future. > You could add to a test like BugzillaTaskDataHandlerTest or whatever test class > is most appropriate (or create a new one and add it to AllBugzillaTests.java). I start with a simple test. Now I want to open the editor from that test to force method sortComments of AbstractRepositoryTaskEditor but didn't get this work. How can help?
Created attachment 88085 [details] mylyn/context/zip
I can create the icons. Nice work Frank! The question I have about the status of this feature for 2.3, is what impact it has on the rest of the task editor UI. While we have always discussed the idea of reversing the order, we have accepted that it has other implications (e.g. New Comment and actions should appear near the last comment). So I'm wondering how those using this and trying it out are using the feature. Are you keeping comments in reverse order? Does it make sense for us to release this without touching the other affected parts of the interaction? I'm not saying that the answer is no, since the button is very obvious and unobtrusive. But clicking it triggers a serious UI change that has other implications that I would like to understand before we release it.
(In reply to comment #44) > I can create the icons. Nice work Frank! > > The question I have about the status of this feature for 2.3, is what impact it > has on the rest of the task editor UI. While we have always discussed the idea > of reversing the order, we have accepted that it has other implications (e.g. > New Comment and actions should appear near the last comment). What is with the original Bugzilla code? Layout is the following New Comment, radiobutton actions, description and the comments. But this meen with the default order the latest comment is at the button of the page. So that new comment can be fare away. The option "Newest to Oldest, but keep Description on the Top" is for my the only one where new comment, newest comment and description are close together. For Mylyn this can be done if we change layout to Description, new Comment, Comments. But where we the place Actions and People. For me Description, new Comment, Actions and People, Comments could work. What do you think? > So I'm wondering > how those using this and trying it out are using the feature. Are you keeping > comments in reverse order? No comment are in the original order only the Control array is changed by the method org.eclipse.swt.widgets.Control.moveAbove(org.eclipse.swt.widgets.Control) > Does it make sense for us to release this without > touching the other affected parts of the interaction? I'm not saying that the > answer is no, since the button is very obvious and unobtrusive. But clicking it > triggers a serious UI change that has other implications that I would like to > understand before we release it. I use the reverse order if I want to know if the latest comment and the description are related. In some Tasks I found that over a number of comments the direction was changed so that the latest comment did not longer match to the description. If I have stuff that is related to an comment (latest or not) I do most of the time use the reply button and delete the commented text after I insert my new text.
BTW, what do you think about to have a sizable scroller control that hold the whole Comment Group. With this it is possible to use the lowest possible space to show up what the user expect to see.
I find the reverse comment ordering particularly useful when browsing incoming changes. Now I have to scroll less often to get to new comments. Rob and I have discussed further optimizations, e.g. putting people next to attributes and moving down attachments but it will require some experimentation to find a good layout that also works well on small screens. On the downside I got confused a few times when a bug had no incomings. Being so used to the standard ordering I looked I scrolled down to the last comment in the list and only notice after reading that it wasn't what I was looking for. I am not sure if it is helpful to have the option to reverse the comment ordering in the task editor or if that should be a global option to always have a predictable order when opening the editor. (In reply to comment #44) > The question I have about the status of this feature for 2.3, is what impact it > has on the rest of the task editor UI. While we have always discussed the idea > of reversing the order, we have accepted that it has other implications (e.g. > New Comment and actions should appear near the last comment). So I'm wondering > how those using this and trying it out are using the feature. I would like us to hold off with any changes of that sort until the editor code is refactored for 3.0. Since the time frame until 2.3 might be too short to converge on the UI we could also make the comment ordering a sandbox option. That would give us the opportunity to gather more feedback before releasing it to a larger user base (after that it's going to be more difficult to change the way the feature works).
(In reply to comment #47) > I find the reverse comment ordering particularly useful when browsing incoming > changes. Now I have to scroll less often to get to new comments. There was an idea to not show collapsed entries and only show expanded entries with the new comments (i.e. have actions on "Comments" section header like "show new", "show last N", "show all". What bugs me about reversed comment order is that it breaks the chronological order and if there is more then one new comment you have to read from the bottom, which doesn't really encourage the reader to understand entire discussion.
fyi Frank, I've disabled comment ordering in the Bugzilla task editor for the 2.3 release. We need to sort out the ui layout issues befor general release. We'll iterate on these as early as possible in the 3.0. I'll make it a sandbox enabled feature in the short term (i.e. install mylyn sandbox and comment ordering becomes enabled in the Bugzilla task editor).
