Defect #9143

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

Added by Isaac Betesh about 6 years ago. Updated over 5 years ago.

Status:ClosedStart date:2011-08-29
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:SCM
Target version:1.3.2
Resolution:Fixed Affected version:1.2.2

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 file Closed 2008-12-22
Duplicated by Redmine - Defect #9440: Escaping issue with inline diff-highlighting in revision ... Closed 2011-10-20

Associated revisions

Revision 8876
Added by Jean-Philippe Lang almost 6 years ago

Fixed that partial diffs are done against html instead of original code (#9143).

History

#1 Updated by Etienne Massip about 6 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?

#2 Updated by Toshi MARUYAMA about 6 years ago

  • Category changed from SCM extra to SCM

#3 Updated by Toshi MARUYAMA about 6 years ago

  • Category deleted (SCM)

#4 Updated by Toshi MARUYAMA about 6 years ago

  • Category set to Text formatting

#5 Updated by Isaac Betesh about 6 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?

#6 Updated by Etienne Massip almost 6 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.

#7 Updated by Jean-Philippe Lang almost 6 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

#8 Updated by Jean-Philippe Lang almost 6 years ago

Fixed in r8876.

#9 Updated by Jean-Philippe Lang over 5 years ago

  • Status changed from Resolved to Closed

Merged.

Also available in: Atom PDF