Community
Participate
Working Groups
Allow the user to enter a filename for a patch that is going to be attached to an issue and provide a more meaningful default. BUGID.patch or BUGID-summary-delimited-by-dashes.patch seem good.
Related: bug 190944 (JIRA ignores description when submitting attachments).
This is relevant to all attachments created from the clipboard.
A meaningful filename could be better, but we would need to work out a suitable default. * BUGID.patch: what we consider the task ID is usualy an internal ID (Bugzilla is the exception). In other issue trackers like JIRA, keys such as JDB-123 can change while the internal ID stays the same. * summary-delimited-by-dashes.patch: summaries can change while the attachment is still valid.
Bumping priority since this we now have bug reports with 10+ attachments all named clipboard.txt that were uploaded through Mylyn.
Copying description from bug 209610: Current situation: If you create a patch to the clipboard and add it to the task, the file is added as clipboard.txt. This is confusing to the commiters that review the patch (all have the same name). Desired situation: The patch should be names like bugId_Name.txt, f.e. 207432_20071112.txt.
*** Bug 209610 has been marked as a duplicate of this bug. ***
It might also make sense to give the uploading party a choice to name the patch as they would when saving it to the filesystem. Otherwise picking up version numbers for a series of patches would be nice too. Let's there already exists a patch named: adds-feature-foo.patch The following ones coulde have an incremental version number: adds-feature-foo-2.patch
Out of curiosity, why do you guys need to save patches and actually care about their names? "Apply path" in the task editor does hide all the low level details.
Eugene, there are more use-cases than applying patches through Mylyn. Even then it is difficult enough to distinguish different attachments. Felix, would you be interested in contributing a patch? Most of the relevant code should be in NewAttachmentWizard. Another bug related to the attachment wizard is here: bug 209572 .
Yeah, I'd be interested in looking into it, if there are no time constraints on it.
It's only a minor enhancement with a low priority and no fixed milestone. Assigning to you.
(In reply to comment #9) > Eugene, there are more use-cases than applying patches through Mylyn. Even then > it is difficult enough to distinguish different attachments. Steffen, sorry, but that was question to original reporters, but since you answered, can you please list the use cases you are referring to? I am still curious to hear why it is difficult to distinguish attachments in Mylyn Task Editor, which shows attachment date and it author. Thanks.
We had issues that had multiple patches addressing several separate concerns in the bug report. Also it is not very convenient to have a meaningless name clipboard.txt suggested as the default when saving the attachment. Moreover the JIRA web ui does not show the date by default. It will only list filenames. Please take a look at this issue for instance that has 13 attachments named clipboard.txt: https://www.limewire.org/jira/browse/CORE-310
I see. Thanks Steffen.
After looking at the code for a bit I have a few questions and a few suggestions for refactoring it: * Implicit dependencies on NewAttachmentWizard in PreviewAttachmentPage. It casts the wizard to this type in createControl. I'm in favor making the dependency explicit by passing in the right type of Wizard in the constructor, or better rely on the LocalAttachment as model and get the preview contents from there encapsulating where the content comes from. * Store the type of attachment in LocalAttachment and try to only pass it around between wizard pages if possible. Also remove the file reading in PreviewAttachmentPage and move it into LocalAttachment as an implementation detail, can also cache the contents there, once read. * Remove checks where InputAttachmentSourcePage.CLIPBOARD_LABEL is used to find out if an attachment is a clipboard attachment. * I would only show the "Attachment File Name" field when the attachment comes from the clipboard, otherwise the control would not be visible. Thanks for your comments, Felix
These suggestions sound great. As long as the involved classes are in internal packages we can make these changes. I'll take a more detailed look later today. Could you attach your context?
+1 for using LocalAttachment as a placeholder, but I don't think it is a good idea to put file reading inside that class. Also before doing these changes it would make sense to finish Willian's patch on bug 195691 which will be conflicting.
Can't attach my context, other projects slipped in and Eclipse hangs whenever I try to remove them from the context. I'll file a separate issue for that.
> Can't attach my context, other projects slipped in and Eclipse hangs whenever I > try to remove them from the context. I'll file a separate issue for that. Yes, removing items from the context can cause excessive refresh, please comment on bug 202817 . > * Implicit dependencies on NewAttachmentWizard in PreviewAttachmentPage. It > casts the wizard to this type in createControl. > > I'm in favor making the dependency explicit by passing in the right type of > Wizard in the constructor, or better rely on the LocalAttachment as model and > get the preview contents from there encapsulating where the content comes from. That sounds exactly right. Sharing an instance of LocalAttachment between the wizard pages will make this dependency unnecessary. > * Store the type of attachment in LocalAttachment and try to only pass it around > between wizard pages if possible. Also remove the file reading in > PreviewAttachmentPage and move it into LocalAttachment as an implementation > detail, can also cache the contents there, once read. That makes sense. Is there a common interface that could be used to abstract access to the data like InputStreams or IStorage? > * Remove checks where InputAttachmentSourcePage.CLIPBOARD_LABEL is used to find > out if an attachment is a clipboard attachment. It would be great if this could abstracted in LocalAttachment. > * I would only show the "Attachment File Name" field when the attachment comes > from the clipboard, otherwise the control would not be visible. I think it would be okay to also show it in the other cases, particularly for JIRA which ignores the description. You can go with the implementation that is easiest and we can iterate on the UI afterwards. Felix, don't worry about other contributions conflicting with these changes. I'll take care of the merging if necessary.
Created attachment 83351 [details] mylyn/context/zip
Implemented as part of bug 222833. All wizard pages now share a common data model and editing of filenames is enabled for JIRA. For attachments from the clipboard the filename still defaults to clipboard.txt. Have we converged on a better default?
Marking this bug as resolved. I have opened bug 239045 for further discussion about to the default filename for attachments created from the clipboard.
