On 5/22/13, Gary Martin <[email protected]> wrote:
> On 22/05/13 16:28, Anze Staric wrote:
>> A number of errors is caused by the following change
>>
>> r1467339 | jdreimann | 2013-04-12 18:33:52 +0200 (Fri, 12 Apr 2013) | 3
>> lines
>>
>> Updated styling of comment history, towards #471.
>> Changed styling of attachment download icon to use background-image
>> instead of an img inserted on the page.
>> Removed outdated screenshot.png because it doesn't appear to be used.
>>
>> Index: trac/trac/wiki/formatter.py
>> ===================================================================
>> --- trac/trac/wiki/formatter.py (revision 1467338)
>> +++ trac/trac/wiki/formatter.py (revision 1467339)
>> @@ -707,7 +707,7 @@
>>           local_url = self.env.project_url or \
>>                       (self.req or self.env).abs_href.base
>>           if not url.startswith(local_url):
>> -            return tag.a(tag.span(u'\u200b', class_="icon"), text,
>> +            return tag.a(text, tag.span(u'\u200b', class_="icon"),
>>
>> If I revert it, only 4 tests fail on python 2.6.
>> IMHO, the only way to fix tests failing because of this change is to
>> change the expected result in tests.
>>
>>
>> Anze
>
> Good spot. Yes, that looks like it explains a lot. We could say that the
> change does not belong in the trac part of the codebase but I imagine we
> may still get the same issue if we override the python code by other means.
>
> Maybe if we want the link position reversed we should just do it in js
> with something like:
>
> $('A.ext-link').each(function(){this.appendChild(this.firstChild,this);});
>

IMO this will unnecessarily over-complicate things just to make test
cases pass . IMO the goal is to build the system with no big effort in
a way agreed by community based on consensus , and test cases are a
tool just serving to the purpose of ensuring that those decisions are
satisfied by source code .

IMO , if <span /> position changes , then just update test cases, thus
making this decision obvious by inspecting the test suite . Also
beware of the fact that this will cause an extra overhead while
performing subsequent merges with Trac trunk .

Otherwise , just revert to what they were before .

What's more important ? A decision has to be made .

-- 
Regards,

Olemis.

Reply via email to