[patch] Linking images not possible for pages with ':' in name

[Dirk Stöcker]

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 :-)

[osimons]

Replying to dstoecker:

Version 0.12.3 (there is no entry for 0.12.3 ATM :-)

There is now… Thanks. ;-)

[Dirk Stöcker]

Any chance this gets fixed?

[Christian Boos]

Let's keep it on the radar, at least.

[Christian Boos]

Note that this has now to play well with #10850, i.e. ":" in attachment filenames.

[Christian Boos]

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).

[Dirk Stöcker]

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.

[Dirk Stöcker]

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

[Christian Boos]

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.

[Dirk Stöcker]

Your wish is my command

macros.

@@ -18 +18 @@
 from itertools import groupby
 import inspect
 import os
+import csv
 import re
 from StringIO import StringIO
 
@@ -578 +579 @@
                         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 :-)

[Christian Boos]

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.

[Dirk Stöcker]

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)

[Christian Boos]

wiki:"Scarlett_O'Hara"?

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

[Dirk Stöcker]

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.

[Dirk Stöcker]

Could this last one please be applied? It is tested for 4 month on JOSM page now any has no drawbacks.

[Christian Boos]

Too late for 1.0.1, but for 1.0.2, promised ;-)

[Dirk Stöcker]

Well, now?

[Christian Boos]

I'll go through all pending 1.0.2 tickets this week.

[Christian Boos]

Suggested fix implemented in r11732, together with a test.

Thanks for your patience!