#10562 closed defect (fixed)
[patch] Linking images not possible for pages with ':' in name
Reported by: | Dirk Stöcker | Owned by: | Christian Boos |
---|---|---|---|
Priority: | normal | Milestone: | 1.0.2 |
Component: | wiki system | Version: | 1.0 |
Severity: | normal | Keywords: | wikipagename |
Cc: | Branch: | ||
Release Notes: |
Image macro can now reach attachments on Wiki pages having a |
||
API Changes: | |||
Internal Changes: |
Description
With Image(wiki:page:name)
it is possible to link to attachements from other pages.
This is not possible when the page name contains a ':'. Encoding the : With %3A does not work as well as a lot of other workarounds I tried.
This is especially disturbing, as the default localization used the ':' to separate translations (like used for a lot of wikis).
Version 0.12.3 (there is no entry for 0.12.3 ATM :-)
Attachments (0)
Change History (19)
comment:1 by , 12 years ago
Version: | → 0.12.3 |
---|
comment:3 by , 12 years ago
Keywords: | wikipagename added |
---|---|
Milestone: | → next-stable-1.0.x |
Let's keep it on the radar, at least.
comment:4 by , 12 years ago
Note that this has now to play well with #10850, i.e. ":" in attachment filenames.
comment:5 by , 12 years ago
While at it, another Image macro enhancement would be to allow for "../" prefix (get the attachment from the parent page, useful for quickly fixing links after moving content to a subpage).
comment:6 by , 12 years ago
I think a minimum fix, which could do no harm would be to allow %3A for encoding. although not really fine, it probably could also fix a bunch of other possible issues.
And probably it only needs a single call to an entity decoder at the right place.
comment:7 by , 12 years ago
Summary: | Linking images not possible for pages with ':' in name → [patch] Linking images not possible for pages with ':' in name |
---|
Following allows to use character encoding for me:
--- wiki/macros.py_ 2012-10-04 23:39:59.557103777 +0200 +++ wiki/macros.py 2012-10-04 23:55:18.907440181 +0200 @@ -32,7 +32,7 @@ from trac.util.datefmt import format_date, from_utimestamp, user_time from trac.util.html import escape, find_element from trac.util.presentation import separated -from trac.util.text import unicode_quote, to_unicode, stripws +from trac.util.text import unicode_quote, unicode_unquote, to_unicode, stripws from trac.util.translation import _, dgettext, cleandoc_ from trac.wiki.api import IWikiMacroProvider, WikiSystem, parse_args from trac.wiki.formatter import ( @@ -589,6 +589,7 @@ desc = url.rsplit('?')[0] elif filespec.startswith('/'): # project-relative params = '' + filespec = unicode_unquote(filespec) if '?' in filespec: filespec, params = filespec.rsplit('?', 1) url = formatter.href(filespec) @@ -599,6 +600,8 @@ # # or intertrac:realm:id realm, id, filename = parts intertrac_target = "%s:%s" % (id, filename) + id = unicode_unquote(id) + filename = unicode_unquote(filename) it = formatter.get_intertrac_url(realm, intertrac_target) if it: url, desc = it @@ -607,6 +610,7 @@ attachment = Resource(realm, id).child('attachment', filename) elif len(parts) == 2: realm, filename = parts + filename = unicode_unquote(filename) if realm in browser_links: # source:path # TODO: use context here as well rev = None
comment:8 by , 12 years ago
As the TracLinks syntax wiki:"page:name" works just fine, I'd rather support that in the Image macro.
I highly suspect that if we start to URL decode the links, we'll get some other surprising behavior.
comment:9 by , 12 years ago
Your wish is my command
--- macros.py_ 2012-10-04 23:39:59.557103777 +0200 +++ macros.py 2012-10-08 21:20:06.669710029 +0200 @@ -18,6 +18,7 @@ from itertools import groupby import inspect import os +import csv import re from StringIO import StringIO @@ -578,7 +579,7 @@ attr[str(key)] = val # will be used as a __call__ kwd # parse filespec argument to get realm and id if contained. - parts = filespec.split(':') + parts = csv.reader([filespec], delimiter=':').next() url = raw_url = desc = None attachment = None if (parts and parts[0] in ('http', 'https', 'ftp')): # absolute
Hmm, I start to like some parts of python :-)
comment:10 by , 12 years ago
The "battery included" part, I suppose? ;-)
Going for the csv module seems a bit heavy handed here… Furthermore, it does work for the use case I've given, but not for the other valid syntax wiki:'page:name'. OTOH, using a regexp here would make things more complicated.
comment:11 by , 12 years ago
Can be fixed as well:
parts = csv.reader([filespec.replace('\'', '"')], delimiter=':').next()
As page names should not have ' inside that should be correct. I was convinced CSV takes care of both quotes itself.
With RE:
parts = re.compile(r'''((?:[^:"']|"[^"]*"|'[^']*')+)''').split(filespec)
comment:12 by , 12 years ago
I'll take the regexp version ;-)
Plus the following:
def recompose_string_list(segments, sep=':'): """Recompose a string list by concatenating all consecutive elements stopping at the separator ones. """ parts = [''] for s in segments: if s == sep: parts.append('') else: parts[-1] += s return parts
comment:13 by , 12 years ago
There is an bug in the RE. This RE actually works correct:
parts = [i.strip('''['"]''') for i in re.compile(r''':(?!(?:[^"':]|[^"']:[^'"])+["'])''').split(filespec)]
It does not care that quotes are actually paired correctly, but I don't want to make it more complex.
comment:14 by , 11 years ago
Could this last one please be applied? It is tested for 4 month on JOSM page now any has no drawbacks.
comment:15 by , 11 years ago
Milestone: | next-stable-1.0.x → 1.0.2 |
---|---|
Owner: | set to |
Status: | new → assigned |
Too late for 1.0.1, but for 1.0.2, promised ;-)
comment:18 by , 11 years ago
Release Notes: | modified (diff) |
---|---|
Resolution: | → fixed |
Status: | assigned → closed |
Suggested fix implemented in r11732, together with a test.
Thanks for your patience!
comment:19 by , 11 years ago
Release Notes: | modified (diff) |
---|
Replying to dstoecker:
There is now… Thanks. ;-)