Posted by jhodgdon on February 21, 2012 at 4:27pm
8 followers
| Project: | Drupal core |
| Version: | 6.x-dev |
| Component: | other |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
| Issue tags: | needs backport to D6, needs backport to D7, Novice |
Issue Summary
In MAINTAINERS.txt files we have a lot of lines like this:
- Dries Buytaert 'dries' <http://drupal.org/user/1>The URLs in these lines are not turning into links. They should.
Comments
#1
Oh and this should have tests in the api_link_documentation test and the web test.
#2
Well, it turns out that we are using the core function _filter_url() to turn URLs into links, so the issue is there. I think this is a won't fix.
#3
<http://...>looks like HTML to _filter_url(), so it avoids it. We can update MAINTAINERS.txt to use different punctuation, or just remove punctuation.#4
Yeah, true, good idea. I'd be in favor of removing the <> around the URLs. It doesn't add to readability. Good Novice project (oh, drumm already tagged, good). And I think we should also similarly patch D6 and D7.
#5
D8 MAINTAINERS.txt updated without <> punctuation.
#6
The patch looks fine to me. When you upload a patch, you need to set the issue status to "needs review" by the way. :)
#7
Backport to D6 and D7. Sorry about the "needs review" status :-)
#8
The last submitted patch, D7-Punctuation-1448326-7.patch, failed testing.
#9
#10
anavarre: if you upload a patch for a different version, see the help text under the "attachments" field to find out how to name it to avoid the "needs work" problem above.
Anyway, I'd like to leave this for a day or two to see if anyone objects, then commit this.
#11
MAINTAINERS.txt is a text file. If your editor is not able to understand
<http://example.com>like it should, fix your editor.#12
It is not "your editor" that is the problem. It is the Drupal 6 URL filter, which is being used to display the URL when the file is displayed on api.drupal.org -- it isn't turning into a link.
Is there really a reason why we shouldn't remove the <> in order to accommodate the limitations of this filter, since at this point there is probably no way it will be improved? Personally, I don't think this punctuation improves the readability of the MAINTAINERS.txt file, and if we need some punctuation, perhaps we can find some alternative punctuation that would keep the readability while still allowing the URL to be displayed as a link on api.drupal.org?
#13
Much better questions are:
1) Why does a text file appear on api.d.o in the first place?
2) Why do we attempt to parse a text file in any way?
3) How were you able to find and access MAINTAINERS.txt on api.d.o at all? (it shouldn't be linked anywhere)
#14
1) We have had .txt files on api.drupal.org for quite a while. Some, such as robots.txt and INSTALL.txt, are arguably part of the Drupal code/documentation base. If MAINTAINERS.txt is not, then why would we include it in the Drupal distribution? We could just maintain it as a page on drupal.org in that case.
2) Not sure historically why are are parsing text files. I've committed a patch to the API module recently that reduces the "parsing" to just turning things into links (e.g., names of files/functions in the Drupal code base, and full URLs).
3) You can find it by typing it into a search.
#15
Reroll/new version of #5 that applies cleanly to D8 HEAD.
#16
The patch in #15 applied cleanly and removed every occurrence of
<>around the URLs (verified manually and by performing a search for both characters against the text in maintainers.txt).Reviewing the patch, I also didn't find any mistakes which might have modified a maintainers name or d.o profile URL.
#17
Seems like this is fixing a legitimate bug for API module, so re-classifying.
Committed and pushed to 8.x. Thanks!
#18
Sorry! This was meant to be backported.
#19
patches for d7 and d6
#20
The last submitted patch, d6-1448326-19.patch, failed testing.
#21
Thanks @jibran! D7 patch looks good.
Since this issue is now against 7.x, to have the D6 patch skip testing (and avoid testbot setting this issue to needs work), end the patch filename with -do-not-test.patch. See https://drupal.org/node/1092232.
#22
Committed and pushed to 7.x. One more time... :)
#23
d6 patch
#24
Thanks! This patch appears to be complete and correct.
#25
I just noticed this very old issue. The patch does not apply now and needs a reroll. Sorry for not getting it committed earlier!
#26
PFA