Community
Participate
Working Groups
The branch filter added in bug 76386 is fine but it could be even better. To keep up with the current Eclipse UI style the filter should look more like TagSelectionDialog used when switching to a branch or merging. It should allow to select from a list, have content assist, etc.
Looking at the latest patch in 76386 I could even live with the new filter as provided there, if the labels are brought into a consistent state: currently one field has "contains", one "containing" and one has no addition. I would use "contains" for all three fields.
Created attachment 192354 [details] moved piece of changes from bug 76386 Remember this patch requires the latest patch from bug 76386
(In reply to comment #1) > I would use "contains" for all three fields. Makes sense to me, but for the Author field "matches" is more appropriate as this is how this criteria works. Or we could change the way it works ;)
Created attachment 193164 [details] Olexiy's patch with minor changes
Created attachment 193165 [details] mylyn/context/zip
I found one minor thing, I hope it's the last one: CVSUIMessages.HistoryFilterDialog_branchname should be renamed to HistoryFilterDialog_branchName to be consistent with other names. Olexiy, could you please update the latest patch? This way we could safely say it's your contribution. Dani, should I open a new bug for comment 0, or are we gonna keep this one open?
(In reply to comment #6) > I found one minor thing, I hope it's the last one: > CVSUIMessages.HistoryFilterDialog_branchname should be renamed to > HistoryFilterDialog_branchName to be consistent with other names. > > Olexiy, could you please update the latest patch? This way we could safely say > it's your contribution. > > Dani, should I open a new bug for comment 0, or are we gonna keep this one > open? I think we can leave it as is since the idea is to add a pattern. What I'd like to see though is F1 help that explains which patterns are supported.
(In reply to comment #7) > What I'd like to see though is F1 help that explains which patterns are supported. Filed bug 342969 for updating the doc. The F1 help for the filter dialog is just one sentence, I guess you meant the "Filtering CVS Revisions in the History View" page in Help.
(In reply to comment #8) > (In reply to comment #7) > > What I'd like to see though is F1 help that explains which patterns are supported. > > Filed bug 342969 for updating the doc. The F1 help for the filter dialog is > just one sentence, I guess you meant the "Filtering CVS Revisions in the > History View" page in Help. The whole F1 help for that dialog needs work: 1. There is a (?) button but that doesn't work (opens tool tip instead of tray) 2. The F1 text does not link to a help page which explains the dialog's options, see e.g. Open Type's F1 help.
Tomek, I would leave the 'branch' filter as is in the latest patch and update Help early during RC1. If there are any improvements planned, like these from comment 0, please raise a separate bug for it and set for 3.8.
Created attachment 194046 [details] rename branchname to branchName (In reply to comment #6) > I found one minor thing, I hope it's the last one: > CVSUIMessages.HistoryFilterDialog_branchname should be renamed to > HistoryFilterDialog_branchName to be consistent with other names. > > Olexiy, could you please update the latest patch? This way we could safely say > it's your contribution. Done
(In reply to comment #10) > Tomek, I would leave the 'branch' filter as is in the latest patch Fixed, but I have used none of the patches provided. > and update Help early during RC1. Already scheduled for RC1, see bug 342969. > If there are any improvements planned, like these from > comment 0, please raise a separate bug for it and set for 3.8. Filed bug 344438.
Tomek, as we are now in the end game (http://www.eclipse.org/eclipse/development/plans/freeze_plan_3_7.php) there are certain rules that must be followed, e.g. for RC1 you need a second committer approving that the bug is worth fixing plus you need a review+ on the patch from a reviewer (approver and reviewer can but need not to be the same person). Of course both of that must happen before a fix is committed to HEAD ;-). Personally, I'd say this is feature work which should not be done during RC* but if Szymon approves it for RC1 and reviews the patch then that's fine with me.
(In reply to comment #13) > Tomek, as we are now in the end game Sorry, forgot about that. I just got back from vacation and thought it's a straightforward fix.
Created attachment 194682 [details] Applied patch
Looks good. Go ahead.
Thanks Sim. Already committed to HEAD at the time of comment 12. Available in builds >=I20110504-0800.