Posted by joachim on October 31, 2009 at 10:57am
| Project: | Drupal.org customizations |
| Version: | 6.x-3.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Issue Summary
Here is a patch that adds a link to any comment attachment that is a patch.
On drupal.org, the link path would be http://drupal.org/patch/apply :)
This would be an extremely useful feature on d.org, as I have lost count of the number of times this happens:
Poster: HALP! a bug!
Maintainer: Here is a patch. Please test.
Poster: What do I do with this file? (or: 'I put the file in my module directory and nothing happens', etc etc.
Maintainer: (has to link to http://drupal.org/patch/apply yet again....)
| Attachment | Size |
|---|---|
| project_issue.patch-help.patch | 3.16 KB |
Comments
#1
I've spoken to dww on IRC and we're both thinking this might be better off in the Drupal.org infrastructure module.
Setting back to 'needs work'.
#2
Oops, wrong project.
#3
Patch on the drupal.org module, 6--1 branch.
#4
Yeah, much better to just have a trivial patch like this to hard-code it in drupalorg.module than trying to make this a generic feature of project_issue. However:
A) We should probably add this link for both .diff and .patch files.
B) This sure is weird:
+ $variables['attachments'][$id]['text'] .= '</a> - <a class="patch-help" href="' . DRUPALORG_PATCH_HANDBOOK_URL . '">Get help with patches';Why the fun with the
</a>hackery? :( It'd be nice to have a more clear comment about what's going on here. Better yet, don't do this weird stuff. ;) Instead, just loop over all the attachments, and if we see any .patch or .diff, we append a description to the attachment table with the link in smaller help-text print. I don't see why we need N links if there are N patches.Thanks!
-Derek
#5
The hackery is necessary as far as I can tell, because there's no other way to get extra stuff into theme_comment_upload_attachments() / template_preprocess_comment_upload_attachments(). It's a while since I looked at this, but there wasn't a cleaner way.
And AFAIK there's no way of getting to the end of the attachments table. It's also fairly rare to have two patches on a single comment, so if anything putting the link after the table is just going to look odd on all the singletons.
See http://drupalcode.org/viewvc/drupal/contributions/modules/comment_upload...
#6
Having spoken on IRC, here's an alternative approach:
- add a description of the attachments table to comment_upload's template file (which alone does absolutely nothing).
- set that when there is at least one patch or diff file.
Would need some CSS in the site theme to make the link in the P smaller, maybe look like grey form description text?
#7
+1 from me, this is a regular request on almost all project issue queues.