dradis / dradis-ce

Dradis Framework: Collaboration and reporting for IT Security teams
https://dradis.com/ce/
GNU General Public License v2.0
668 stars 190 forks source link

Diff for first revision displays incorrectly #462

Closed em77 closed 2 years ago

em77 commented 5 years ago

Steps to reproduce

Go into an issue which has no revision history (just the initial creation). Make a small edit. Now go to view the revision history and there is an unhelpful diff which doesn't just highlight the changed line but interprets all the content as having changed. Does does not seem to happen for later revisions, only for the first one.

Expected behavior

The same changes should give the exact same diff result regardless of what revision the change was part of.

Actual behavior

It behaves differently on the first revision.

System configuration

Dradis version: 3.12

Ruby version: 2.4.1

OS version: MacOS 10.13.6

etdsoft commented 5 years ago

For me the interesting question is: why is revision #1 different? Is it because it's default content (e.g. from the setup command?

More broadly, the question we should be asking is: how do we want to treat "new line" only changes? If two users, one in Linux one in Windows are accessing the project, and in each revision they're making a minor tweak, do we want the differences in "\r\n" | "\n" to show in the diff or not?

em77 commented 5 years ago

So it looks like this is still an problem for created issues (not just sample ones) but my PR didn't solve the problem on user-created issues, only the sample ones. I'll need to look at this more later.

In my view, for the purposes of viewing the diff, the newline differences across platforms should be harmonized because otherwise it's more unclear what's actually different and the diff feature is more unusable when these differences occur since it's not highlighting the most relevant human-made changes. What do you think?

MattBudz commented 2 years ago

Closing due to inactivity. This is noted but not on our roadmap to address. Feel free to re-open with new/additional info.