Skip to content

Commit

Permalink
Settings/Code Style/JavaDoc: fix highlighting of changed fragments
Browse files Browse the repository at this point in the history
* do not highlight whole line, if there are found inner fragments
-- before this change, whole modified line was highlighted. Now only changed whitespaces are highlighted.
-- it was broken in 7af486c (DiffMarkup was not intended to be used like this)
* Use new diff API
  • Loading branch information
AMPivovarov committed May 19, 2016
1 parent cb6b52b commit 30d0673
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 139 deletions.
Expand Up @@ -15,76 +15,62 @@
*/
package com.intellij.application.options;

import com.intellij.openapi.Disposable;
import com.intellij.diff.comparison.ComparisonManager;
import com.intellij.diff.comparison.ComparisonPolicy;
import com.intellij.diff.comparison.DiffTooBigException;
import com.intellij.diff.fragments.DiffFragment;
import com.intellij.diff.fragments.LineFragment;
import com.intellij.openapi.diagnostic.Logger;
import com.intellij.openapi.diff.DiffContent;
import com.intellij.openapi.diff.actions.MergeOperations;
import com.intellij.openapi.diff.impl.ComparisonPolicy;
import com.intellij.openapi.diff.impl.fragments.Fragment;
import com.intellij.openapi.diff.impl.fragments.FragmentHighlighterImpl;
import com.intellij.openapi.diff.impl.fragments.LineFragment;
import com.intellij.openapi.diff.impl.highlighting.DiffMarkup;
import com.intellij.openapi.diff.impl.highlighting.FragmentSide;
import com.intellij.openapi.diff.impl.processing.TextCompareProcessor;
import com.intellij.openapi.diff.impl.util.TextDiffType;
import com.intellij.openapi.diff.impl.util.TextDiffTypeEnum;
import com.intellij.openapi.editor.Document;
import com.intellij.openapi.editor.ex.EditorEx;
import com.intellij.openapi.editor.markup.GutterIconRenderer;
import com.intellij.openapi.editor.markup.SeparatorPlacement;
import com.intellij.openapi.fileEditor.FileEditor;
import com.intellij.openapi.progress.DumbProgressIndicator;
import com.intellij.openapi.util.TextRange;
import com.intellij.openapi.util.text.StringUtil;
import com.intellij.util.diff.FilesTooBigForDiffException;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;

import java.util.*;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

/**
* Allows to calculate difference between two versions of document (before and after code style setting value change).
* <p/>
* Not thread-safe.
*
* @author Denis Zhdanov
* @since 10/14/10 2:44 PM
*/
public class ChangesDiffCalculator implements Disposable {
private static final Logger LOG = Logger.getInstance("#com.intellij.application.options.ChangesDiffCalculator");
private final BaseMarkup myOldMarkup = new BaseMarkup(FragmentSide.SIDE1, this);
private final ChangesCollector myNewMarkup = new ChangesCollector(this);
private final TextCompareProcessor myCompareProcessor = new TextCompareProcessor(ComparisonPolicy.DEFAULT);
public class ChangesDiffCalculator {
private static final Logger LOG = Logger.getInstance(ChangesDiffCalculator.class);

public Collection<TextRange> calculateDiff(@NotNull final Document beforeDocument, @NotNull final Document currentDocument) {
myNewMarkup.ranges.clear();
myOldMarkup.document = beforeDocument;
myNewMarkup.document = currentDocument;
public static List<TextRange> calculateDiff(@NotNull Document beforeDocument, @NotNull Document currentDocument) {
CharSequence beforeText = beforeDocument.getCharsSequence();
CharSequence currentText = currentDocument.getCharsSequence();

List<LineFragment> lineFragments;
try {
lineFragments = myCompareProcessor.process(beforeDocument.getText(), currentDocument.getText());
ComparisonManager manager = ComparisonManager.getInstance();
List<LineFragment> lineFragments = manager.compareLinesInner(beforeText, currentText, ComparisonPolicy.DEFAULT,
DumbProgressIndicator.INSTANCE);

List<TextRange> modifiedRanges = new ArrayList<>();

for (LineFragment lineFragment : lineFragments) {
int fragmentStartOffset = lineFragment.getStartOffset2();
int fragmentEndOffset = lineFragment.getEndOffset2();

List<DiffFragment> innerFragments = lineFragment.getInnerFragments();
if (innerFragments != null) {
for (DiffFragment innerFragment : innerFragments) {
int innerFragmentStartOffset = fragmentStartOffset + innerFragment.getStartOffset2();
int innerFragmentEndOffset = fragmentStartOffset + innerFragment.getEndOffset2();
modifiedRanges.add(calculateChangeHighlightRange(currentText, innerFragmentStartOffset, innerFragmentEndOffset));
}
}
else {
modifiedRanges.add(calculateChangeHighlightRange(currentText, fragmentStartOffset, fragmentEndOffset));
}
}

return modifiedRanges;
}
catch (FilesTooBigForDiffException e) {
catch (DiffTooBigException e) {
LOG.info(e);
return Collections.emptyList();
}

final FragmentHighlighterImpl fragmentHighlighter = new FragmentHighlighterImpl(myOldMarkup, myNewMarkup);
for (Iterator<LineFragment> iterator = lineFragments.iterator(); iterator.hasNext();) {
LineFragment fragment = iterator.next();
fragmentHighlighter.setIsLast(!iterator.hasNext());
fragment.highlight(fragmentHighlighter);
}

List<TextRange> ranges = new ArrayList<>(myNewMarkup.ranges);
for (TextRange range : myNewMarkup.ranges) {
if (range.getStartOffset() >= currentDocument.getTextLength()) {
continue;
}
ranges.add(calculateChangeHighlightRange(currentDocument, range));
}

return ranges;
}

/**
Expand All @@ -93,12 +79,8 @@ public Collection<TextRange> calculateDiff(@NotNull final Document beforeDocumen
* Thus, when comparing whitespace sequences of different length, we always highlight rightmost whitespaces
* (while general algorithm gives no warranty on this case, and usually highlights leftmost whitespaces).
*/
private static TextRange calculateChangeHighlightRange(Document currentDocument, TextRange range) {
CharSequence text = currentDocument.getCharsSequence();

int startOffset = range.getStartOffset();
int endOffset = range.getEndOffset();

@NotNull
private static TextRange calculateChangeHighlightRange(@NotNull CharSequence text, int startOffset, int endOffset) {
if (startOffset == endOffset) {
while (startOffset < text.length() && text.charAt(startOffset) == ' ') {
startOffset++;
Expand All @@ -116,80 +98,4 @@ private static TextRange calculateChangeHighlightRange(Document currentDocument,

return new TextRange(startOffset, endOffset);
}

@Override
public void dispose() {
}

/**
* Base {@link DiffMarkup} implementation that does nothing.
*/
@SuppressWarnings({"ConstantConditions"})
private static class BaseMarkup extends DiffMarkup {

public Document document;
private final FragmentSide mySide;

BaseMarkup(@NotNull FragmentSide side, @NotNull Disposable parentDisposable) {
super(null, parentDisposable);
mySide = side;
}

@Override
public FragmentSide getSide() {
return mySide;
}

@Override
public DiffContent getContent() {
return null;
}

@Override
public EditorEx getEditor() {
return null;
}

@Nullable
@Override
public Document getDocument() {
return document;
}

@Override
public void addLineMarker(int line, TextDiffType type, SeparatorPlacement separatorPlacement) {
}

@Override
public void addAction(MergeOperations.Operation operation, int lineStartOffset) {
}

@Override
public void highlightText(@NotNull Fragment fragment, GutterIconRenderer gutterIconRenderer) {
}


@Override
public FileEditor getFileEditor() {
return null;
}
}

private static class ChangesCollector extends BaseMarkup {
private static final Set<TextDiffTypeEnum> INTERESTED_DIFF_TYPES = EnumSet.of(TextDiffTypeEnum.INSERT, TextDiffTypeEnum.DELETED, TextDiffTypeEnum.CHANGED);

public final List<TextRange> ranges = new ArrayList<TextRange>();

ChangesCollector(@NotNull Disposable parentDisposable) {
super(FragmentSide.SIDE2, parentDisposable);
}

@Override
public void highlightText(@NotNull Fragment fragment, GutterIconRenderer gutterIconRenderer) {
TextRange currentRange = fragment.getRange(FragmentSide.SIDE2);
if (INTERESTED_DIFF_TYPES.contains(fragment.getType())) {
ranges.add(currentRange);
}
}
}
}
Expand Up @@ -35,7 +35,6 @@
import com.intellij.openapi.project.Project;
import com.intellij.openapi.project.ProjectUtil;
import com.intellij.openapi.ui.OnePixelDivider;
import com.intellij.openapi.util.Disposer;
import com.intellij.openapi.util.TextRange;
import com.intellij.psi.PsiDocumentManager;
import com.intellij.psi.PsiFile;
Expand Down Expand Up @@ -70,7 +69,6 @@ public abstract class CodeStyleAbstractPanel implements Disposable {

private static final Logger LOG = Logger.getInstance("#com.intellij.application.options.CodeStyleXmlPanel");

private final ChangesDiffCalculator myDiffCalculator = new ChangesDiffCalculator();
private final List<TextRange> myPreviewRangesToHighlight = new ArrayList<TextRange>();

private final Editor myEditor;
Expand Down Expand Up @@ -99,7 +97,6 @@ protected CodeStyleAbstractPanel(@Nullable Language defaultLanguage,
@Nullable CodeStyleSettings currentSettings,
@NotNull CodeStyleSettings settings)
{
Disposer.register(this, myDiffCalculator);
myCurrentSettings = currentSettings;
mySettings = settings;
myDefaultLanguage = defaultLanguage;
Expand Down Expand Up @@ -294,7 +291,7 @@ private void highlightChanges(Document beforeReformat) {
MarkupModel markupModel = myEditor.getMarkupModel();
markupModel.removeAllHighlighters();

myPreviewRangesToHighlight.addAll(myDiffCalculator.calculateDiff(beforeReformat, myEditor.getDocument()));
myPreviewRangesToHighlight.addAll(ChangesDiffCalculator.calculateDiff(beforeReformat, myEditor.getDocument()));

if (!myPreviewRangesToHighlight.isEmpty()) {
myEndHighlightPreviewChangesTimeMillis = System.currentTimeMillis() + TIME_TO_HIGHLIGHT_PREVIEW_CHANGES_IN_MILLIS;
Expand Down
@@ -0,0 +1,71 @@
/*
* Copyright 2000-2016 JetBrains s.r.o.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.intellij.application.options;

import com.intellij.openapi.editor.impl.DocumentImpl;
import com.intellij.openapi.util.TextRange;
import com.intellij.testFramework.LightPlatformTestCase;
import org.jetbrains.annotations.NotNull;

import java.util.Arrays;
import java.util.List;

public class ChangesDiffCalculatorTest extends LightPlatformTestCase {
public void testEmpty() {
doTest("", "");

doTest("a b c", "a b c");

doTest("a\nb\nc\n", "a\nb\nc\n");
}

public void testSimple() {
doTest("", "X",
new TextRange(0, 1));

doTest("a B c", "a X c",
new TextRange(2, 3));

doTest("while()", "while ()",
new TextRange(5, 6));

doTest("else {", "else\n {",
new TextRange(4, 8));

doTest("while ()", "while()",
new TextRange(5, 5));

doTest("if\n then", "if\n\n then",
new TextRange(3, 4));
}

public void testWhitespaceSequences() {
doTest("X Y", "X Y",
new TextRange(3, 3));

doTest("X Y", "X Y",
new TextRange(3, 6));
}

private static void doTest(@NotNull String before, @NotNull String current, @NotNull TextRange... expectedRanges) {
DocumentImpl beforeDocument = new DocumentImpl(before);
DocumentImpl currentDocument = new DocumentImpl(current);

List<TextRange> actualRanges = ChangesDiffCalculator.calculateDiff(beforeDocument, currentDocument);

assertEquals(Arrays.asList(expectedRanges), actualRanges);
}
}

0 comments on commit 30d0673

Please sign in to comment.