Diff shows tab as only 7 spaces when part of intraline changes

[Tim Hatch]

Evident in t.e.o in the changeset r4591, confirmed an issue on trunk. It's because of the way expandtabs() is being done in source:trunk/trac/versioncontrol/diff.py​ lines 176 and 180 and the \0 from markup_intraline_changes counts as a character, but is then removed to insert the tag, making the expanded line-that-used-to-have-a-tab now only start with 7 spaces.

For example, edit '\ta' to ' a' and you'd expect them to end up the same length in the diff. The way I'd like to fix this is with a custom version of expandtabs that takes a list of zero-width characters such as ('\0', '\1').

[Tim Hatch]

See attachment:t4557.diff​ for a rough draft. I think it ought to have some tests too.

Ideas?

[Christian Boos]

Another approach, seems to work as well

[Christian Boos]

Your patch made me wonder if we couldn't get rid of expandtabs completely, as we already to some processing on the line with the space_re and htmlify.

The proof-of-concept patch seems to work fine, for a simple test at least ;)

[Tim Hatch]

Minor bits just noticed with both patches:

• attachment:t4557.diff​ doesn't need to use the special expandtabs in the tag == 'equal' case, it could use the c-optimized regular one. Also the special expandtabs is a nop if there is not \t in the text, this is actually the common case for most of my files so should probably be checked
• attachment:no_expandtabs-r4597.diff​ has an issue because tabstops hop to the next tabstop, they aren't just "worth 8 spaces". For example, '1234567\t' would be 8 characters wide, as would '1234\t'`.

[Christian Boos]

Replying to thatch:

tabstops hop to the next tabstop, they aren't just "worth 8 spaces".

oops :) True.

Ok, so yours with the 2 optimizations (check if \t present and check for 'equal) seems the way to go.

[Tim Hatch]

Applied modified patch in r4603.