Skip to content

Commit

Permalink
Paragraph list trim fix (#795)
Browse files Browse the repository at this point in the history 
* bug: getVisibleParagraph does not work with blank lines
* trim paragraphs using reference instead of object comparison (issues #777 and #788)
  • Loading branch information
@JFormDesigner @Jugen
JFormDesigner authored and Jugen committed Feb 20, 2019
1 parent fa72c42 commit 6d49399
Show file tree
Hide file tree
Showing 2 changed files with 225 additions and 1 deletion.
152 changes: 152 additions & 0 deletions richtextfx/src/integrationTest/java/org/fxmisc/richtext/api/VisibleParagraphTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
package org.fxmisc.richtext.api;

import org.fxmisc.richtext.InlineCssTextAreaAppTest;
import org.fxmisc.richtext.model.Paragraph;
import org.junit.Test;
import org.junit.Assert;
import org.reactfx.collection.LiveList;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;

public class VisibleParagraphTest extends InlineCssTextAreaAppTest {

@Test
public void get_first_visible_paragraph_index_with_non_blank_lines() {
String[] lines = {
"abc",
"def",
"ghi"
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(0, area.firstVisibleParToAllParIndex());
assertEquals(2, area.lastVisibleParToAllParIndex());
assertEquals(1, area.visibleParToAllParIndex(1));
}
@Test
public void get_last_visible_paragraph_index_with_non_blank_lines() {
String[] lines = {
"abc",
"def",
"ghi"
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(2, area.lastVisibleParToAllParIndex());
}
@Test
public void get_specific_visible_paragraph_index_with_non_blank_lines() {
String[] lines = {
"abc",
"def",
"ghi"
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(2, area.visibleParToAllParIndex(2));
}
@Test
public void get_first_visible_paragraph_index_with_all_blank_lines() {
String[] lines = {
"",
"",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(0, area.firstVisibleParToAllParIndex());
}
@Test
public void get_last_visible_paragraph_index_with_all_blank_lines() {
String[] lines = {
"",
"",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(2, area.lastVisibleParToAllParIndex());
}
@Test
public void get_specific_visible_paragraph_index_with_all_blank_lines() {
String[] lines = {
"",
"",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(2, area.visibleParToAllParIndex(2));
}

@Test
public void get_first_visible_paragraph_index_with_some_blank_lines() {
String[] lines = {
"abc",
"",
"",
"",
"def",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(0, area.firstVisibleParToAllParIndex());
}
@Test
public void get_last_visible_paragraph_index_with_some_blank_lines() {
String[] lines = {
"abc",
"",
"",
"",
"def",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(5, area.lastVisibleParToAllParIndex());
}
@Test
public void get_specific_visible_paragraph_index_with_some_blank_lines() {
String[] lines = {
"abc",
"",
"",
"",
"def",
""
};
interact(() -> {
area.setWrapText(true);
stage.setWidth(120);
area.replaceText(String.join("\n", lines));
});
assertEquals(3, area.visibleParToAllParIndex(3));
}
}
74 changes: 73 additions & 1 deletion richtextfx/src/main/java/org/fxmisc/richtext/model/GenericEditableStyledDocumentBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.ListIterator;

import org.reactfx.EventSource;
import org.reactfx.EventStream;
Expand All @@ -19,6 +20,8 @@
import org.reactfx.collection.QuasiListModification;
import org.reactfx.collection.SuspendableList;
import org.reactfx.collection.UnmodifiableByDefaultLiveList;
import org.reactfx.util.Tuple2;
import org.reactfx.util.Tuples;
import org.reactfx.value.SuspendableVal;
import org.reactfx.value.Val;

Expand All @@ -43,7 +46,7 @@ protected Subscription observeInputs() {
return parChangesList.subscribe(list -> {
ListChangeAccumulator<Paragraph<PS, SEG, S>> accumulator = new ListChangeAccumulator<>();
for (MaterializedListModification<Paragraph<PS, SEG, S>> mod : list) {
mod = mod.trim();
mod = trim(mod);

// add the quasiListModification itself, not as a quasiListChange, in case some overlap
accumulator.add(QuasiListModification.create(mod.getFrom(), mod.getRemoved(), mod.getAddedSize()));
Expand Down Expand Up @@ -217,4 +220,73 @@ private void updateMulti(
parChangesList.push(parChanges);
});
}

/**
* Copy of org.reactfx.collection.MaterializedListModification.trim()
* that uses reference comparison instead of equals().
*/
private MaterializedListModification<Paragraph<PS, SEG, S>> trim(MaterializedListModification<Paragraph<PS, SEG, S>> mod) {
return commonPrefixSuffixLengths(mod.getRemoved(), mod.getAdded()).map((pref, suff) -> {
if(pref == 0 && suff == 0) {
return mod;
} else {
return MaterializedListModification.create(
mod.getFrom() + pref,
mod.getRemoved().subList(pref, mod.getRemovedSize() - suff),
mod.getAdded().subList(pref, mod.getAddedSize() - suff));
}
});
}

/**
* Copy of org.reactfx.util.Lists.commonPrefixSuffixLengths()
* that uses reference comparison instead of equals().
*/
private static Tuple2<Integer, Integer> commonPrefixSuffixLengths(List<?> l1, List<?> l2) {
int n1 = l1.size();
int n2 = l2.size();

if(n1 == 0 || n2 == 0) {
return Tuples.t(0, 0);
}

int pref = commonPrefixLength(l1, l2);
if(pref == n1 || pref == n2) {
return Tuples.t(pref, 0);
}

int suff = commonSuffixLength(l1, l2);

return Tuples.t(pref, suff);
}

/**
* Copy of org.reactfx.util.Lists.commonPrefixLength()
* that uses reference comparison instead of equals().
*/
private static int commonPrefixLength(List<?> l, List<?> m) {
ListIterator<?> i = l.listIterator();
ListIterator<?> j = m.listIterator();
while(i.hasNext() && j.hasNext()) {
if(i.next() != j.next()) {
return i.nextIndex() - 1;
}
}
return i.nextIndex();
}

/**
* Copy of org.reactfx.util.Lists.commonSuffixLength()
* that uses reference comparison instead of equals().
*/
private static int commonSuffixLength(List<?> l, List<?> m) {
ListIterator<?> i = l.listIterator(l.size());
ListIterator<?> j = m.listIterator(m.size());
while(i.hasPrevious() && j.hasPrevious()) {
if(i.previous() != j.previous()) {
return l.size() - i.nextIndex() - 1;
}
}
return l.size() - i.nextIndex();
}
}

0 comments on commit 6d49399

Please # to comment.