Bug 570850: [R-DataEditor] Improve generatation of filter expression
- Make filter more robust by always specifying is.na in filter
expression if another filter option is active
Change-Id: I4fd8eaf186b0b6e3d69f9c837e308503eb9d85cd
diff --git a/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/IntervalVariableFilter.java b/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/IntervalVariableFilter.java
index a6ae29e..d9b31c5 100644
--- a/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/IntervalVariableFilter.java
+++ b/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/IntervalVariableFilter.java
@@ -155,48 +155,55 @@
@Override
protected @Nullable String createFilter(final String varExpression) {
final var minMaxData= this.minMaxData;
+ final Number lower;
+ final Number upper;
if (minMaxData == null
- || this.selectedLowerValue.getValue() == null
- || this.selectedUpperValue.getValue() == null) {
+ || (lower= this.selectedLowerValue.getValue()) == null
+ || (upper= this.selectedUpperValue.getValue()) == null) {
return null;
}
final StringBuilder sb= new StringBuilder();
sb.append('(');
- {
- final Number lower= this.selectedLowerValue.getValue();
- if (isGreater(lower, minMaxData.get(MIN_IDX))) {
- sb.append(varExpression);
- sb.append(" >= "); //$NON-NLS-1$
- sb.append(lower);
- }
+
+ int num= 0;
+ int na= 0;
+
+ if (isGreater(lower, minMaxData.get(MIN_IDX))) {
+ sb.append(varExpression);
+ sb.append(" >= "); //$NON-NLS-1$
+ sb.append(lower);
+ num++;
}
- { final Number upper= this.selectedUpperValue.getValue();
- if (isSmaller(upper, minMaxData.get(MAX_IDX))) {
- if (sb.length() > 1) {
- sb.append(" & "); //$NON-NLS-1$
- }
- sb.append(varExpression);
- sb.append(" <= "); //$NON-NLS-1$
- sb.append(upper);
+ if (isSmaller(upper, minMaxData.get(MAX_IDX))) {
+ if (sb.length() > 1) {
+ sb.append(" & "); //$NON-NLS-1$
}
+ sb.append(varExpression);
+ sb.append(" <= "); //$NON-NLS-1$
+ sb.append(upper);
+ num++;
}
-// if (this.minMaxData.getLogi(NA_IDX)) {
- final Boolean na= (!minMaxData.getLogi(NA_IDX))
- || this.selectedNA.getValue();
- if (na) {
- if (sb.length() > 1) {
+ if (minMaxData.getLogi(NA_IDX)) {
+ na= (this.selectedNA.getValue()) ? 1 : -1;
+ }
+ if (num > 0 || na < 0) {
+ if (na >= 0) {
+ if (num > 1) {
sb.insert(0, '(');
- sb.append(") | "); //$NON-NLS-1$
- sb.append("is.na(").append(varExpression).append(')'); //$NON-NLS-1$
+ sb.append(')');
}
+ if (num > 0) {
+ sb.append(" | "); //$NON-NLS-1$
+ }
+ sb.append("is.na(").append(varExpression).append(')'); //$NON-NLS-1$
}
- else { // !na
- if (sb.length() > 1) {
+ else {
+ if (num > 0) {
sb.append(" & "); //$NON-NLS-1$
}
sb.append("!is.na(").append(varExpression).append(')'); //$NON-NLS-1$
}
-// }
+ }
sb.append(')');
return (sb.length() <= 2) ? "" : sb.toString(); //$NON-NLS-1$
}
diff --git a/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/LevelVariableFilter.java b/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/LevelVariableFilter.java
index fc50486..9534fa4 100644
--- a/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/LevelVariableFilter.java
+++ b/r/org.eclipse.statet.r.ui/src/org/eclipse/statet/internal/r/ui/datafilter/LevelVariableFilter.java
@@ -158,7 +158,7 @@
num++;
}
}
- if (num > 0 || na == 1) {
+ if (num > 0 || na > 0) {
if (na >= 0) {
if (num > 0) {
sb.append(" | "); //$NON-NLS-1$
@@ -166,9 +166,11 @@
sb.append("is.na(").append(varExpression).append(')'); //$NON-NLS-1$
}
else {
- if (num > 0) {
+ if (num > 1) {
sb.insert(0, '(');
sb.append(')');
+ }
+ if (num > 0) {
sb.append(" & "); //$NON-NLS-1$
}
sb.append("!is.na(").append(varExpression).append(')'); //$NON-NLS-1$