Bugzilla – Full Text Bug Listing
| Summary: | (PatchAttached) Allow for keeping commit comments permanent | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Maik Schreiber <blizzy-keyword-eclipse_bugs.ba215a> | ||||||||||||
| Component: | CVS | Assignee: | platform-cvs-inbox <platform-cvs-inbox> | ||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||
| Severity: | enhancement | ||||||||||||||
| Priority: | P3 | Keywords: | helpwanted | ||||||||||||
| Version: | 3.0 | ||||||||||||||
| Target Milestone: | 3.2 M2 | ||||||||||||||
| Hardware: | All | ||||||||||||||
| OS: | All | ||||||||||||||
| Whiteboard: | |||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
Maik Schreiber
Created attachment 24301 [details]
patch for org.eclipse.team.cvs.ui to add canned comments feature
This patch adds canned comments to org.eclipse.team.cvs.ui. Most notably it
adds a "Add as canned comment" check box to the commit comment dialog. Please
note that there's no way to delete canned comments yet.
Created attachment 24310 [details]
new patch version
New version of the patch. This version changes the checkbox to a more intuitive
"Keep permanent". Each previously entered commit comment can now be made
permanent by selecting the comment, then activating the checkbox. A newly
entered comment can be made permanent right away by activating the same
checkbox. Comments that are permanent already can be made non-permanent by
selecting them and deactivating the checkbox.
Perhaps permanent comments should be marked in the combobox? What do you think?
I like the idea. However, I would prefer if you provided a preference page for managing the template comments. Adding buttons for managing the comments to the commit dialog will just confuse most users. We can point the users from the dialog to the preference page using the help. As for the appearance in the Combo box, I think it would be enough to have the template comments appear first, followed by a separator of some kind and then the comment history. I will consider a patch with these changes (of course, there is also room for negotiation if you have other ideas). Sorry about the delay, but here goes... 1. Rename "permanent comments" to "comment templates": Good idea. Personally I don't have that much use for templates, but I guess folks at Apache could use that (think "Submitted by: / Approved by: " etc.). Nice. 2. What do you think about removing the checkbox, and adding some linked text label, that when clicked will bring you to the templates prefs page directly? That way it's not as intrusive because you don't have to cancel the whole commit process but can edit the templates "right there". When finished, just select it from the combo and off you go. Sounds quite nice to me :) Sounds good. Just to summarize, what is required is 1) A combo list that shows an MRU and templates comments in such a way that the two are easily distinguishable. 2) A preference page for managing commit templates 3) A link from the commit dialog to the commit template preference page. The easy approach is to use the help to mention the preferences page. A deluxe solution involves adding a button or link to the commit dialog that opens a separate dialog that contains the commit templatge preference page. Created attachment 24894 [details] new patch as per comment #5 This patch implements the features mentioned in comment #5. Minor glitches (I've added TODO's): - Combo box shows "TEMPL:" where a comment is actually a template. This must be changed to not showing "TEMPL:" and adding a divider between the templates and the regular comments. - When saving the templates list XML file fails when clicking "OK" in the preference page, no error message is displayed. Instead, the TeamException is swallowed silently. Just to make sure: The latest patch is not incremental, rendering the first two patches obsolete. Please do not apply those. Thanks for the patch. I'll have a look when I have time (sometime in the next few weeks) Alright. I can't release this patch. You need to add a suitable copyright to any new files. Two of the three new files have an IBM copyright (do you work for IBM?). A third file is missing a copyright altogether. Here is a link that describes what the copyright should look like: http://www.eclipse.org/legal/copyrightandlicensenotice.html Also, the following things need to be fixed. 1) Neumonics for labels and buttons need to be added (so that ALT key navigation works). 2) The TEMPL prefix needs to be externalized and changed it to Template: as TEMPL is not something the average user will undrstand. 3) There is a bug when a comment is added using the Configure link. Basically, the item gets added but selecting it puts the text that was previously at that index in the comment field. Other than that, the patch looks good. So, I need the copyrights, neumonics and NLSed Template prefix (although I would still prefer a separator;-). As for the link, I'm not actually convinced I want to have it there. It is a good indicator but it would set a precedent (i.e. I haven't seen any other dialogs that link to preference pages this way). I'll need to consult with the UI guys about that one. Oh, I wasn't aiming to have it applied the fast way :) Just trying things out and see what you say. I will fix the things you mentioned. Re separator in combo: The only thing I can think of is a "-----------", which I do think would be rather ugly. Aside from that, the user could select it (having no effect of course). Unfortunately, I can't see how to add an option that is disabled :-/ As for the link in the dialog, the Java project properties dialog has one, too. For example, the "Java Code Style" page there has a link to the respective global preference page. Not sure if that counts, though :) Re copyrights: Two of the files are actually copied & pasted, with _extremely_ minor modifications. I'm not quite sure what to do with those. I could add myself as contributor, but I think that's asking too much already ;) Created attachment 24963 [details] new patch as per comment #10 Things fixed: - Added copyrights/contributor statements. For two of the new files that are really just copied and pasted, I haven't modified the copyright statements at all. - Mnemonics added. - "TEMPL:" prefix changed to "Template:" and NLS'ed. I don't seem to be able to reproduce the bug you mentioned, though. I can add new templates using the link and select them in the combo just fine. Perhaps you can provide me with the exact steps to reproduce that? Created attachment 25364 [details]
new version of the patch
This version of the patch fixes the bug you mentioned where entering a comment
could erase a comment template having the same text.
Patch released. I had to make a few modifications mostly due to changes made since you generated the patch. That's great news. What would be the next integration build where I can test the new feature? The next integration build should be on Tuesday. Verified in I20050920-0010 |