Confusing error message in TSVN Merge upon comment change

[Stefan Hett]

Hi,

Steps to reproduce the behavior:

1. Make sure that TSVNMerge is set to ignore Comments:

2. Modify a C/C++ file and make sure it only differs in a comment.
3. Bring up the diff view for that file.

Actual result:
Following error popup comes up.

Expected behavior:
No error pop up at all.

Note that the incorrect text in the popup adds to the confusion (i.e. it says that whitespace changes were detected, while actually only changes in comments were different).
Please understand that IMO there should be no popup at all in this case. The difference is clearly marked in the TSVNMerge window (even if ignore comments is set) so there should be little point in presenting an additional popup on top on all of this.

-- 
Regards,
Stefan Hett, Developer/Administrator

EGOSOFT GmbH, Heidestrasse 4, 52146 Würselen, Germany
Tel: +49 2405 4239970, www.egosoft.com
Geschäftsführer: Bernd Lehahn, Handelsregister Aachen HRB 13473

[Stefan]

Expected behavior:

No error pop up at all.

Note that the incorrect text in the popup adds to the confusion (i.e. it says that whitespace changes were detected, while actually only changes in comments were different).
Please understand that IMO there should be no popup at all in this case. The difference is clearly marked in the TSVNMerge window (even if ignore comments is set) so there should be little point in presenting an additional popup on top on all of this.

Yes, that's a bug.

Problem is now, how to fix it. Because as you said, "the difference is clearly marked ... even if ignore comments is set" - and that's what's actually wrong. So if I fix this and TMerge would not show any changes in comments (because it doesn't show non-whitespace changes in comments, it only shows whitespace changes in ignored comments), I would also have to show a popup, but then clearly state that there are changes in comments.

So if I want to fix this right, I have to show a popup...

Stefan

[Stefan Hett]

Hm... I see... In this case I guess it'd be ok to show the popup with the corrected message then. Ultimately what I could imagine is a different behavior when selecting to ignore comments (basically the way WinMerge handles this case):
- still visualize the changes in comments but in a fainter (i.e. less bright) color (compared to the normal color marking the diffs)
- when going to next/previous diff, skip over the ignored ones

But I'm sure this is way beyond the scope for a simple bugfix, so personally I'm fine if a correct popup would be displayed.

[Stefan]

Hm... I see... In this case I guess it'd be ok to show the popup with the corrected message then. Ultimately what I could imagine is a different behavior when selecting to ignore comments (basically the way WinMerge handles this case):
- still visualize the changes in comments but in a fainter (i.e. less bright) color (compared to the normal color marking the diffs)
- when going to next/previous diff, skip over the ignored ones

But I'm sure this is way beyond the scope for a simple bugfix, so personally I'm fine if a correct popup would be displayed.

So best without adding too much complexity (and most of all, without getting a massive performance hit or double the amount of memory required) I'll implement it like this:

Note: "filter" can mean the "ignore comments" filter but also the regex filters

- if filtered lines differ even with the filter applied, they're shown the same way as non-filtered lines. Meaning the inline diff still shows the diffs of the filtered parts

  --> because for the inline-diff to ignore the filtered parts we would need double the memory because we'd have to store the filtered lines as well as the original lines

- if filtered lines are identical with the filter applied, they're shown with a faint colored background, and next/prev-diff skips over them. The inline-diff will not be shown on those lines, except for the two-line preview at the bottom

Stefan

[Stefan Hett]

Sounds great. Thanks for taking care about this so quickly.