From 6577287e26d74cd40bcba8c7005fe3a0e2dadc10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= Date: Sat, 4 Mar 2017 20:45:05 +0100 Subject: [PATCH] Improve the diff when a paragraph is replaced by another one --- openslides/motions/static/js/motions/diff.js | 37 +++++++++++++++++++- tests/karma/motions/diff.service.test.js | 9 ++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/openslides/motions/static/js/motions/diff.js b/openslides/motions/static/js/motions/diff.js index 6ad1578b8..f52da92ec 100644 --- a/openslides/motions/static/js/motions/diff.js +++ b/openslides/motions/static/js/motions/diff.js @@ -854,6 +854,29 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi return str.replace(/^\s+/g, '').replace(/\s+$/g, '').replace(/ {2,}/g, ' '); }; + /** + * Calculates the ratio of the text affected by inline diff + * From 0 (no changes at all) to 1 (everything has changed) + * + * @param html + * @returns {number} + * @private + */ + this._calcChangeRatio = function(html) { + var lengthChanged = 0; + + html = html.replace(/(.*?)<\/del>/gi, function() { lengthChanged += arguments[1].length; return ""; }); + html = html.replace(/(.*?)<\/ins>/gi, function() { lengthChanged += arguments[1].length; return ""; }); + html = html.replace(/<.*?>/, "").trim(); + + var lengthRemaining = html.length; + if (lengthRemaining === 0 && lengthChanged === 0) { + return 0; + } else { + return (lengthChanged / (lengthChanged + lengthRemaining)); + } + }; + /** * * @param {string} html @@ -861,8 +884,20 @@ angular.module('OpenSlidesApp.motions.diff', ['OpenSlidesApp.motions.lineNumberi * @private */ this._diffDetectBrokenDiffHtml = function(html) { + // If a regular HTML tag is enclosed by INS/DEL, the HTML is broken var match = html.match(/<(ins|del)><[^>]*><\/(ins|del)>/gi); - return (match !== null && match.length > 0); + if (match !== null && match.length > 0) { + return true; + } + + // If too much of the text is changed, it's better to separate the old from new new version, + // otherwise the result looks strange + if (this._calcChangeRatio(html) > 0.66) { + return true; + } + + // If non of the conditions up to now is met, we consider the diff as being sane + return false; }; this._diffParagraphs = function(oldText, newText, lineLength, firstLineNumber) { diff --git a/tests/karma/motions/diff.service.test.js b/tests/karma/motions/diff.service.test.js index 0fae9e570..747f46e5e 100644 --- a/tests/karma/motions/diff.service.test.js +++ b/tests/karma/motions/diff.service.test.js @@ -368,7 +368,14 @@ describe('linenumbering', function () { var before = "

Ihr könnt ohne Sorge fortgehen.'Da meckerte die Alte und machte sich getrost auf den Weg.

", after = ""; var diff = diffService.diff(before, after); - expect(diff).toBe("

Ihr könnt ohne Sorge fortgehen.'Da meckerte die Alte und machte sich getrost auf den Weg.

"); + expect(diff).toBe("

Ihr könnt ohne Sorge fortgehen.'Da meckerte die Alte und machte sich getrost auf den Weg.

"); + }); + + it('does not perform inline diff if there are too many changes', function () { + var before = "

Dann kam er zurück, klopfte an die Hausthür und rief 'macht auf, ihr lieben Kinder, eure Mutter ist da und hat jedem von Euch etwas mitgebarcht.' Aber der Wolf hatte seine schwarze Pfote in das Fenster gelegt, das sahen die Kinder und riefen

", + after = "

(hier: Missbrauch von bewusstseinsverändernde Mittel - daher Zensiert)

"; + var diff = diffService.diff(before, after); + expect(diff).toBe('

Dann kam er zurück, klopfte an die Hausthür und rief \'macht auf, ihr lieben Kinder, eure Mutter ist da und hat jedem von Euch etwas mitgebarcht.\' Aber der Wolf hatte seine schwarze Pfote in das Fenster gelegt, das sahen die Kinder und riefen

(hier: Missbrauch von bewusstseinsverändernde Mittel - daher Zensiert)

'); }); }); });