Skip to content

Commit

Permalink
in response to review
Browse files Browse the repository at this point in the history 
JabRef#7788 (review)

- more use of StringUtil.isNullOrEmpty
- private final XTextRangeCompare cmp;
- List<V> partition = partitions.computeIfAbsent(partitionKey, _key -> new ArrayList<>());
- visualSort does not throw WrappedTargetException, NoDocumentException
- set renamed to comparableMarks
  • Loading branch information
@antalk2
antalk2 committed Jul 7, 2021
1 parent 9712162 commit fe5a4bc
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/model/openoffice/ootext/OOFormat.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ public static OOText paragraph(OOText ootext) {
/**
* Format an OO cross-reference showing the target's page number as label to a reference mark.
*/
public static OOText formatReferenceToPageNumberOfReferenceMark(String referencMarkName) {
String string = String.format("<oo:referenceToPageNumberOfReferenceMark target=\"%s\">", referencMarkName);
public static OOText formatReferenceToPageNumberOfReferenceMark(String referenceMarkName) {
String string = String.format("<oo:referenceToPageNumberOfReferenceMark target=\"%s\">", referenceMarkName);
return OOText.fromString(string);
}
}
5 changes: 2 additions & 3 deletions src/main/java/org/jabref/model/openoffice/ootext/OOText.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ private OOText(String data) {
this.data = data;
}

/* null input is passed through */
/** @return null for null input, otherwise the argument wrapped into a new OOText */
public static OOText fromString(String string) {
if (string == null) {
return null;
}
return new OOText(string);
}

/* null input is passed through */
/** @return null for null input, otherwise the string inside the argument */
public static String toString(OOText ootext) {
if (ootext == null) {
return null;
Expand All @@ -38,7 +38,6 @@ public String toString() {
return data;
}

/* Object.equals */
@Override
public boolean equals(Object object) {

Expand Down
9 changes: 5 additions & 4 deletions src/main/java/org/jabref/model/openoffice/ootext/OOTextIntoOO.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.jabref.model.openoffice.uno.UnoCast;
import org.jabref.model.openoffice.uno.UnoCrossRef;
import org.jabref.model.openoffice.util.OOPair;
import org.jabref.model.strings.StringUtil;

import com.sun.star.awt.FontSlant;
import com.sun.star.awt.FontStrikeout;
Expand Down Expand Up @@ -181,7 +182,7 @@ public static void write(XTextDocument doc, XTextCursor position, OOText ootext)
String endTagName = m.group(1);
String startTagName = m.group(2);
String attributeListPart = m.group(3);
boolean isStartTag = (endTagName == null) || "".equals(endTagName);
boolean isStartTag = StringUtil.isNullOrEmpty(endTagName);
String tagName = isStartTag ? startTagName : endTagName;
Objects.requireNonNull(tagName);

Expand Down Expand Up @@ -231,7 +232,7 @@ public static void write(XTextDocument doc, XTextCursor position, OOText ootext)
switch (key) {
case "oo:ParaStyleName":
// <p oo:ParaStyleName="Standard">
if (value != null && !value.equals("")) {
if (!StringUtil.isNullOrEmpty(value)) {
if (setParagraphStyle(cursor, value)) {
// Presumably tested already:
LOGGER.debug(String.format("oo:ParaStyleName=\"%s\" failed", value));
Expand Down Expand Up @@ -689,7 +690,7 @@ private static List<OOPair<String, Object>> setCharStrikeout(short value) {
// CharStyleName
private static List<OOPair<String, Object>> setCharStyleName(String value) {
List<OOPair<String, Object>> settings = new ArrayList<>();
if (value != null && value != "") {
if (!StringUtil.isNullOrEmpty(value)) {
settings.add(new OOPair<>(CHAR_STYLE_NAME, value));
} else {
LOGGER.warn("setCharStyleName: received null or empty value");
Expand All @@ -708,7 +709,7 @@ private static List<OOPair<String, Object>> setCharLocale(Locale value) {
* Locale from string encoding: language, language-country or language-country-variant
*/
private static List<OOPair<String, Object>> setCharLocale(String value) {
if (value == null || "".equals(value)) {
if (StringUtil.isNullOrEmpty(value)) {
throw new java.lang.IllegalArgumentException("setCharLocale \"\" or null");
}
String[] parts = value.split("-");
Expand Down
36 changes: 21 additions & 15 deletions src/main/java/org/jabref/model/openoffice/rangesort/RangeSort.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,21 +12,29 @@
import com.sun.star.text.XText;
import com.sun.star.text.XTextRangeCompare;

/**
* RangeSort provides sorting based on XTextRangeCompare, which only provides comparison
* between XTextRange values within the same XText.
*/
public class RangeSort {

/*
* Sort within a partition
/**
* Compare two RangeHolders (using RangeHolder.getRange()) within an XText.
*
* Note: since we only look at the ranges, this comparison is generally not consistent with
* `equals` on the RangeHolders. Probably should not be used for key comparison in
* TreeMap&lt;RangeHolder&gt; or Set&lt;RangeHolder&gt;
*
*/

public static class HolderComparatorWithinPartition implements Comparator<RangeHolder> {

XTextRangeCompare cmp;
private final XTextRangeCompare cmp;

HolderComparatorWithinPartition(XText text) {
cmp = UnoCast.cast(XTextRangeCompare.class, text).get();
}

/*
/**
* Assumes a and b belong to the same XText as cmp.
*/
@Override
Expand All @@ -35,7 +43,7 @@ public int compare(RangeHolder a, RangeHolder b) {
}
}

/*
/**
* Sort a list of RangeHolder values known to share the same getText().
*
* Note: RangeHolder.getRange() is called many times.
Expand All @@ -48,10 +56,9 @@ public static <V extends RangeHolder> void sortWithinPartition(List<V> rangeHold
rangeHolders.sort(new HolderComparatorWithinPartition(text));
}

/*
* Partitioning
/**
* Represent a partitioning of RangeHolders by XText
*/

public static class RangePartitions<V extends RangeHolder> {
private final Map<XText, List<V>> partitions;

Expand All @@ -61,11 +68,7 @@ public RangePartitions() {

public void add(V holder) {
XText partitionKey = holder.getRange().getText();
List<V> partition = partitions.get(partitionKey);
if (partition == null) {
partition = new ArrayList<>();
partitions.put(partitionKey, partition);
}
List<V> partition = partitions.computeIfAbsent(partitionKey, _key -> new ArrayList<>());
partition.add(holder);
}

Expand All @@ -74,6 +77,9 @@ public List<List<V>> getPartitions() {
}
}

/**
* Partition RangeHolders by the corresponding XText.
*/
public static <V extends RangeHolder> RangePartitions<V> partitionRanges(List<V> holders) {
RangePartitions<V> result = new RangePartitions<>();
for (V holder : holders) {
Expand All @@ -82,7 +88,7 @@ public static <V extends RangeHolder> RangePartitions<V> partitionRanges(List<V>
return result;
}

/*
/**
* Note: RangeHolder.getRange() is called many times.
*/
public static <V extends RangeHolder> RangePartitions<V> partitionAndSortRanges(List<V> holders) {
Expand Down
25 changes: 6 additions & 19 deletions src/main/java/org/jabref/model/openoffice/rangesort/RangeSortVisual.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@
import java.util.Collections;
import java.util.List;

import org.jabref.model.openoffice.uno.NoDocumentException;
import org.jabref.model.openoffice.uno.UnoScreenRefresh;

import com.sun.star.awt.Point;
import com.sun.star.lang.WrappedTargetException;
import com.sun.star.text.XTextDocument;
import com.sun.star.text.XTextRange;
import com.sun.star.text.XTextViewCursor;
Expand Down Expand Up @@ -38,10 +36,7 @@ public class RangeSortVisual {
*/
public static <T> List<RangeSortable<T>> visualSort(List<RangeSortable<T>> inputs,
XTextDocument doc,
FunctionalTextViewCursor fcursor)
throws
WrappedTargetException,
NoDocumentException {
FunctionalTextViewCursor fcursor) {

final int inputSize = inputs.size();

Expand All @@ -60,27 +55,19 @@ public static <T> List<RangeSortable<T>> visualSort(List<RangeSortable<T>> input
}
fcursor.restore(doc);

if (positions.size() != inputSize) {
throw new IllegalStateException("visualSort: positions.size() != inputSize");
}

// order by position
ArrayList<ComparableMark<RangeSortable<T>>> set = new ArrayList<>(inputSize);
ArrayList<ComparableMark<RangeSortable<T>>> comparableMarks = new ArrayList<>(inputSize);
for (int i = 0; i < inputSize; i++) {
RangeSortable<T> input = inputs.get(i);
set.add(new ComparableMark<>(positions.get(i),
comparableMarks.add(new ComparableMark<>(positions.get(i),
input.getIndexInPosition(),
input));
}
Collections.sort(set, RangeSortVisual::compareTopToBottomLeftToRight);

if (set.size() != inputSize) {
throw new IllegalStateException("visualSort: set.size() != inputSize");
}
Collections.sort(comparableMarks, RangeSortVisual::compareTopToBottomLeftToRight);

// collect ordered result
List<RangeSortable<T>> result = new ArrayList<>(set.size());
for (ComparableMark<RangeSortable<T>> mark : set) {
List<RangeSortable<T>> result = new ArrayList<>(comparableMarks.size());
for (ComparableMark<RangeSortable<T>> mark : comparableMarks) {
result.add(mark.getContent());
}

Expand Down

0 comments on commit fe5a4bc

Please sign in to comment.