Project

General

Profile

Actions

Defect #9143

closed

Partial diff comparison should be done on actual code, not on html

Added by Isaac Betesh over 12 years ago. Updated about 12 years ago.

Status:
Closed
Priority:
Normal
Category:
SCM
Target version:
Start date:
2011-08-29
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

Semicolons were mysteriously appearing in code diffs in the repository, but only when viewed in Redmine. Upon investigation, it became clear that this is because the semicolon was not part of the diff, but was just after it. Since part of the line that changed is enclosed in <span> tags, when the change ends in the old and new versions with distinct HTML special chars, the semicolon terminating the special chars is treated as if it hadn't changed.
Observe, in the example below, how > and & should each be treated as a single unit, but is instead handled one char at a time, making it appear that "&gt" became "&amp" and that the ";" didn't change.

<tr> 
  <th class="line-num">47</th> 
  <th class="line-num"></th> 
  <td class="line-code diff_out"> 
    <pre>void DoSomething(<span>std::auto_ptr&lt;MyClass&gt</span>; myObj)
</pre> 
  </td> 
</tr> 

<tr> 
  <th class="line-num"></th> 
  <th class="line-num">47</th> 
  <td class="line-code diff_in"> 
    <pre>void DoSomething(<span>const MyClass&amp</span>; myObj)
</pre> 
  </td> 
</tr>

I suspect this is a result of the fact that the code in the repository is first rendered as html, and then passed to the engine that analyzes where the changes are. This should be done in the opposite order.


Related issues

Related to Redmine - Feature #2371: character encoding for attachment fileClosedToshi MARUYAMA2008-12-22

Actions
Has duplicate Redmine - Defect #9440: Escaping issue with inline diff-highlighting in revision diff viewClosed2011-10-20

Actions
Actions #1

Updated by Etienne Massip over 12 years ago

  • Category set to SCM extra

I can't reproduce with r6753; could you please attach an example file?

And what about the format of the file downloaded via the "Unified diff" link on the bottom of the page?

Finally, do you have any plugins installed which could interfere?

Actions #2

Updated by Toshi MARUYAMA over 12 years ago

  • Category changed from SCM extra to SCM
Actions #3

Updated by Toshi MARUYAMA over 12 years ago

  • Category deleted (SCM)
Actions #4

Updated by Toshi MARUYAMA over 12 years ago

  • Category set to Text formatting
Actions #5

Updated by Isaac Betesh over 12 years ago

The semicolon does not appear in the unified diff when viewed in Tortoise diff, Notepad++ or Chrome. It also does not appear when I view the diff from our git server. It only appears in html when using Redmine.

Unfortunately, I cannot attach a code sample. The sample in the description above demonstrates the issue clearly. Could you describe the steps by which you attempted to reproduce it?

Actions #6

Updated by Etienne Massip over 12 years ago

  • Category changed from Text formatting to SCM
  • Status changed from New to Confirmed
  • Target version set to Candidate for next minor release
  • Affected version (unused) set to 1.2.2
  • Affected version set to 1.2.2

Indeed, something is wrong with source:/trunk/lib/redmine/unified_diff.rb#L149 and #line* methods.

Actions #7

Updated by Jean-Philippe Lang about 12 years ago

  • Subject changed from when viewing diffs in repositories, comparison should be done on actual code, not on html to Partial diff comparison should be done on actual code, not on html
  • Status changed from Confirmed to Resolved
  • Assignee set to Jean-Philippe Lang
  • Target version changed from Candidate for next minor release to 1.3.2
  • Resolution set to Fixed
Actions #8

Updated by Jean-Philippe Lang about 12 years ago

Fixed in r8876.

Actions #9

Updated by Jean-Philippe Lang about 12 years ago

  • Status changed from Resolved to Closed

Merged.

Actions

Also available in: Atom PDF