Edgewall Software
Modify

Opened 14 years ago

Closed 14 years ago

#2029 closed defect (fixed)

Wiki Links with no description should not strip the prefix for external links

Reported by: Christian Boos Owned by: Christian Boos
Priority: normal Milestone: 0.9
Component: wiki system Version: 0.9b1
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:

Description

The implementation for #867 ([wiki:Sometext] -> Sometext)
had a bad side-effect for external links: ([http://host] -> //host).

I'll propose a patch for fixing this.

PS: Also contained in the patch is the possibility to add a ] in the link description, by enclosing the description in '...' or "..." quotes, like one can already do for the target.

Attachments (1)

external_link_fix.patch (3.0 KB ) - added by Christian Boos 14 years ago.
The initial patch

Download all attachments as: .zip

Change History (6)

by Christian Boos, 14 years ago

Attachment: external_link_fix.patch added

The initial patch

comment:1 by Christian Boos, 14 years ago

Status: newassigned

Forgot to "accept" the ticket I created.

(see #2045)

comment:2 by Christopher Lenz, 14 years ago

I don't understand this change completely:

  • formatter.py

     
    289289        target = fullmatch.group('ltgt')
    290290        if target and target[0] in "'\"":
    291291            target = target[1:-1]
    292         label = fullmatch.group('label') or target
     292        label = fullmatch.group('label')
     293        if not label:
     294            if target:
     295                label = target.startswith('//') and ns+':'+target or target
     296            else:
     297                label = ns
     298        if label and label[0] in "'\"":
     299            label = label[1:-1]
    293300        rel = fullmatch.group('rel')
    294301        if rel:
    295302            return self._make_relative_link(rel, label or rel)

Why would target be empty? I.e. in what case would the ns be used as label? When removing the quotes around the label, why the if label check? Shouldn't the label always be set at that point?

For removing the quotes, I think the following code would be better (as in more readable):

    if label and label[0] in ("'", '"'):
        label = label[1:-1]

Finally, I'd also expand the and/or expression:

    if target:
        label = target
        if label.startswith('//'):
            label = ns + ':' + label
    else:
        ...

(But again, I don't see why target would be empty.)

comment:3 by Christian Boos, 14 years ago

There is a least one situation where the target can be empty: [search: Search Trac]Search Trac

This corresponds to #1937, which would be fixed by the above patch.

comment:4 by Christopher Lenz, 14 years ago

Okay, so if you integrate the syntax recommendations I made above, please apply the patch. I'm planning to release 0.9b2 today (or maybe tomorrow).

It would probably also be a good idea to add some comments to the blocks that aren't so obvious ;-).

comment:5 by Christian Boos, 14 years ago

Resolution: fixed
Status: assignedclosed

Reworked patch applied in r2268. Additional unit tests added in r2269.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to as closed The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.