mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-11 12:53:59 +00:00
Merge pull request #379 from vangel76/fix/lookup-blank-result
Functions: lookups keep an empty target cell blank (#374)
This commit is contained in:
8 files changed
+78
-29
No files matched your search
@@ -203,9 +203,9 @@ impl<'h> Evaluator<'h> {
|
||||
let index = self.value(&args[0]);
|
||||
if let Value::Array(_) = index {
|
||||
let Some(spec) = gridcraft_functions::lookup("CHOOSE") else { return err(CellError::Name) };
|
||||
let mut evaluated = vec![Arg { value: index, from_ref: false }];
|
||||
let mut evaluated = vec![Arg::val(index)];
|
||||
for argument in &args[1..] {
|
||||
evaluated.push(Arg { value: self.value(argument), from_ref: false });
|
||||
evaluated.push(Arg::val(self.value(argument)));
|
||||
}
|
||||
let mut context = FnCtx { ev: self, criteria: None, lambda: None };
|
||||
return gridcraft_functions::call(spec, &evaluated, &mut context).pipe(Ev::V);
|
||||
@@ -351,7 +351,9 @@ impl<'h> Evaluator<'h> {
|
||||
}
|
||||
ev => self.deref(ev),
|
||||
};
|
||||
evaluated.push(Arg { value, from_ref });
|
||||
// Only an argument left out (`f(a,,b)`) is omitted; one that computes a blank is given.
|
||||
let blank_value = !from_ref && value == Value::Empty && lambda_at != Some(i) && !matches!(a, Expr::Missing);
|
||||
evaluated.push(Arg { value, from_ref, blank_value });
|
||||
}
|
||||
let mut ctx = FnCtx { ev: self, criteria, lambda };
|
||||
gridcraft_functions::call(spec, &evaluated, &mut ctx).pipe(Ev::V)
|
||||
@@ -414,7 +416,7 @@ impl<'h> Evaluator<'h> {
|
||||
let (Some(sum), Some(count)) = (gridcraft_functions::lookup("SUM"), gridcraft_functions::lookup("COUNT")) else {
|
||||
return Err(err(CellError::Name));
|
||||
};
|
||||
let args = [Arg { value, from_ref }];
|
||||
let args = [Arg { value, from_ref, blank_value: false }];
|
||||
if kind != Agg::Count {
|
||||
match gridcraft_functions::call(sum, &args, &mut FnCtx { ev: self, criteria: None, lambda: None }) {
|
||||
Value::Number(n) => acc.sum += n,
|
||||
|
||||
@@ -203,7 +203,7 @@ impl<'h> Evaluator<'h> {
|
||||
let horizontal = l.range.height() == 1 && l.range.width() > 1;
|
||||
let same_len = if vertical { r.range.height() == l.range.height() } else { r.range.width() == l.range.width() };
|
||||
if !matches!(key, Value::Array(_)) && (vertical || horizontal) && same_len {
|
||||
let mut xargs = vec![Arg { value: key, from_ref: false }, Arg { value: self.area_value(&l), from_ref: true }];
|
||||
let mut xargs = vec![Arg::val(key), Arg::reference(self.area_value(&l))];
|
||||
// An empty match_mode before a given search_mode is the default (0), not the end.
|
||||
let given = |i: usize| args.get(i).is_some_and(|e| !matches!(e, Expr::Missing));
|
||||
let last = if given(5) {
|
||||
@@ -215,10 +215,11 @@ impl<'h> Evaluator<'h> {
|
||||
};
|
||||
for i in 4..=last {
|
||||
match args.get(i) {
|
||||
Some(Expr::Missing) | None => xargs.push(Arg { value: Value::Number(0.0), from_ref: false }),
|
||||
Some(Expr::Missing) | None => xargs.push(Arg::val(Value::Number(0.0))),
|
||||
Some(e) => {
|
||||
let value = self.value(e);
|
||||
xargs.push(Arg { value, from_ref: false });
|
||||
let blank_value = value == Value::Empty;
|
||||
xargs.push(Arg { value, from_ref: false, blank_value });
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1826,6 +1826,54 @@ fn trim_references() {
|
||||
assert_eq!(t.get("K20"), Value::Error(CellError::Calc));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn lookup_of_an_empty_cell_stays_blank() {
|
||||
// #374: B1 (the value for key "k") is empty.
|
||||
let mut t = T::new();
|
||||
t.set("A1", "k");
|
||||
t.set("A2", "j");
|
||||
t.set("B2", "v");
|
||||
for (at, f, want) in [
|
||||
("D2", "=\"x\"&VLOOKUP(\"k\",A1:B2,2,0)&\"y\"", Value::from("xy")),
|
||||
("D3", "=CONCATENATE(\"x\",VLOOKUP(\"k\",A1:B2,2,0),\"y\")", Value::from("xy")),
|
||||
("D4", "=ISBLANK(XLOOKUP(\"k\",A1:A2,B1:B2))", Value::Bool(true)),
|
||||
("D5", "=\"x\"&XLOOKUP(\"k\",A1:A2,B1:B2)&\"y\"", Value::from("xy")),
|
||||
// A bare lookup still shows 0, like a reference to an empty cell.
|
||||
("D8", "=VLOOKUP(\"k\",A1:B2,2,0)", Value::Number(0.0)),
|
||||
// Arithmetic still reads the blank as 0.
|
||||
("D9", "=VLOOKUP(\"k\",A1:B2,2,0)+1", Value::Number(1.0)),
|
||||
// A blank lookup given as an argument is still given, not omitted.
|
||||
("D10", "=FILTER(1,FALSE,VLOOKUP(\"k\",A1:B2,2,FALSE))", Value::Number(0.0)),
|
||||
("D11", "=XLOOKUP(\"z\",A1:A2,B1:B2,VLOOKUP(\"k\",A1:B2,2,0))", Value::Number(0.0)),
|
||||
// The same holds per element when the call is lifted over an array of blank lookups.
|
||||
("D12", "=CEILING.MATH(6.7,VLOOKUP(\"k\",A1:B2,2,0))", Value::Number(0.0)),
|
||||
("D13", "=SUM(CEILING.MATH(6.7,VLOOKUP({\"k\",\"k\"},A1:B2,2,0)))", Value::Number(0.0)),
|
||||
("D14", "=FLOOR.MATH(6.7,VLOOKUP(\"k\",A1:B2,2,0))", Value::Number(0.0)),
|
||||
("D15", "=SUM(FLOOR.MATH(6.7,VLOOKUP({\"k\",\"k\"},A1:B2,2,0)))", Value::Number(0.0)),
|
||||
] {
|
||||
t.set(at, f);
|
||||
assert_eq!(t.get(at), want, "{f}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn blank_lookup_values_match_excel_windows_observation() {
|
||||
// Excel 365 Windows 2609, build 16.0.20430.20032, black-box check 2026-10-13:
|
||||
// A1="k", B1 and A2 genuinely empty.
|
||||
// All three value lookups preserve blankness inside ISBLANK/concatenation.
|
||||
let mut t = T::new();
|
||||
t.set("A1", "k");
|
||||
for (at, f, want) in [
|
||||
("D1", "=ISBLANK(VLOOKUP(\"k\",A1:B1,2,FALSE))", Value::Bool(true)),
|
||||
("D2", "=ISBLANK(HLOOKUP(\"k\",A1:A2,2,FALSE))", Value::Bool(true)),
|
||||
("D3", "=ISBLANK(LOOKUP(\"k\",A1:A1,B1:B1))", Value::Bool(true)),
|
||||
("D4", "=\"x\"&HLOOKUP(\"k\",A1:A2,2,FALSE)&\"y\"", Value::from("xy")),
|
||||
] {
|
||||
t.set(at, f);
|
||||
assert_eq!(t.get(at), want, "{f}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cell_width_is_the_column_width_dialog_value_rounded() {
|
||||
// #735: Excel 365 values. Widths in px as `home.columnWidth {"chars"}` sets them
|
||||
|
||||
@@ -91,7 +91,7 @@ fn na(_a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
|
||||
fn n_fn(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
|
||||
// N(range) reads the range's first cell, but a computed array (N(A1:A3>0), N({...})) is
|
||||
// converted element by element, so N can stand in for `--` inside SUMPRODUCT.
|
||||
if let Some(Arg { value: Value::Array(arr), from_ref: false }) = a.first() {
|
||||
if let Some(Arg { value: Value::Array(arr), from_ref: false, .. }) = a.first() {
|
||||
let data = arr.data.iter().map(|v| n_of(v.clone()).unwrap_or_else(Value::Error)).collect();
|
||||
return Array::new(arr.rows, arr.cols, data).map(Value::from).ok_or(CellError::Calc);
|
||||
}
|
||||
|
||||
@@ -87,16 +87,19 @@ pub use stat::{aggregate_values, aggregate_values_k};
|
||||
pub struct Arg {
|
||||
pub value: Value,
|
||||
pub from_ref: bool,
|
||||
/// A written argument whose value came out blank (a lookup of an empty cell): given, not
|
||||
/// omitted, although its value is `Value::Empty`.
|
||||
pub blank_value: bool,
|
||||
}
|
||||
|
||||
impl Arg {
|
||||
/// A literal (non-reference) argument.
|
||||
pub fn val(value: impl Into<Value>) -> Arg {
|
||||
Arg { value: value.into(), from_ref: false }
|
||||
Arg { value: value.into(), from_ref: false, blank_value: false }
|
||||
}
|
||||
/// An argument that came from a cell or range reference.
|
||||
pub fn reference(value: impl Into<Value>) -> Arg {
|
||||
Arg { value: value.into(), from_ref: true }
|
||||
Arg { value: value.into(), from_ref: true, blank_value: false }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -50,12 +50,7 @@ fn reject_multi_cell_refs(spec: &FnSpec, args: &[Arg]) -> Option<Vec<Arg>> {
|
||||
if !args.iter().enumerate().any(|(i, a)| multi(i, a)) {
|
||||
return None;
|
||||
}
|
||||
Some(
|
||||
args.iter()
|
||||
.enumerate()
|
||||
.map(|(i, a)| if multi(i, a) { Arg { value: Value::Error(CellError::Value), from_ref: false } } else { a.clone() })
|
||||
.collect(),
|
||||
)
|
||||
Some(args.iter().enumerate().map(|(i, a)| if multi(i, a) { Arg::val(Value::Error(CellError::Value)) } else { a.clone() }).collect())
|
||||
}
|
||||
|
||||
/// Lookups whose every call searches a lookup vector from its start, so a call lifted over `n`
|
||||
@@ -120,6 +115,8 @@ pub(crate) fn call_lifted(spec: &FnSpec, args: &[Arg], ctx: &mut dyn Ctx) -> Val
|
||||
{
|
||||
let v = arr.get_broadcast(r, c);
|
||||
slot.value = if let Value::Array(inner) = &v { inner.data.first().cloned().unwrap_or(Value::Empty) } else { v };
|
||||
// A blank element of a written array argument is given, as a scalar blank is.
|
||||
slot.blank_value = !slot.from_ref && matches!(slot.value, Value::Empty);
|
||||
}
|
||||
}
|
||||
let out = (spec.imp)(&cell_args, ctx);
|
||||
|
||||
@@ -132,11 +132,6 @@ fn row(a: &Array, r: usize) -> Vec<Value> {
|
||||
(0..a.cols).map(|c| a.get(r, c).cloned().unwrap_or(Value::Empty)).collect()
|
||||
}
|
||||
|
||||
/// A looked-up cell value: blanks read as 0 like a reference to an empty cell.
|
||||
fn cell_result(v: Value) -> Value {
|
||||
if matches!(v, Value::Empty) { Value::Number(0.0) } else { v }
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------------------------
|
||||
// VLOOKUP / HLOOKUP / LOOKUP / MATCH
|
||||
|
||||
@@ -160,7 +155,9 @@ fn vh_lookup(args: &[Arg], vertical: bool) -> R<Value> {
|
||||
let pos = if approx { bsearch_last(&x, &keys, false) } else { find_exact(&x, &keys, true, false) };
|
||||
let pos = pos.ok_or(CellError::NA)?;
|
||||
let v = if vertical { table.get(pos, idx) } else { table.get(idx, pos) };
|
||||
Ok(cell_result(v.cloned().unwrap_or(Value::Empty)))
|
||||
// An empty target stays blank, as in Excel: ISBLANK sees it as blank and `&` /
|
||||
// CONCATENATE treat it as ""; only a formula's final result displays 0 (#374).
|
||||
Ok(v.cloned().unwrap_or(Value::Empty))
|
||||
}
|
||||
|
||||
fn vlookup(a: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
|
||||
@@ -193,18 +190,18 @@ fn lookup(args: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
|
||||
} else {
|
||||
None
|
||||
};
|
||||
return v.map(cell_result).ok_or(CellError::NA);
|
||||
return v.ok_or(CellError::NA);
|
||||
}
|
||||
// Array form: search the first row of a wide array, else the first column; return the
|
||||
// matching entry from the last row / column.
|
||||
if la.cols > la.rows {
|
||||
let keys = row(&la, 0);
|
||||
let pos = bsearch_last(&x, &keys, false).ok_or(CellError::NA)?;
|
||||
Ok(cell_result(la.get(la.rows - 1, pos).cloned().unwrap_or(Value::Empty)))
|
||||
Ok(la.get(la.rows - 1, pos).cloned().unwrap_or(Value::Empty))
|
||||
} else {
|
||||
let keys = column(&la, 0);
|
||||
let pos = bsearch_last(&x, &keys, false).ok_or(CellError::NA)?;
|
||||
Ok(cell_result(la.get(pos, la.cols - 1).cloned().unwrap_or(Value::Empty)))
|
||||
Ok(la.get(pos, la.cols - 1).cloned().unwrap_or(Value::Empty))
|
||||
}
|
||||
}
|
||||
|
||||
@@ -325,13 +322,13 @@ fn xlookup(args: &[Arg], _c: &mut dyn Ctx) -> R<Value> {
|
||||
if vertical {
|
||||
let r = row(&ra, p);
|
||||
if r.len() == 1 {
|
||||
return Ok(cell_result(r.into_iter().next().unwrap_or(Value::Empty)));
|
||||
return Ok(r.into_iter().next().unwrap_or(Value::Empty));
|
||||
}
|
||||
array_val(1, r.len(), r)
|
||||
} else {
|
||||
let c = column(&ra, p);
|
||||
if c.len() == 1 {
|
||||
return Ok(cell_result(c.into_iter().next().unwrap_or(Value::Empty)));
|
||||
return Ok(c.into_iter().next().unwrap_or(Value::Empty));
|
||||
}
|
||||
array_val(c.len(), 1, c)
|
||||
}
|
||||
@@ -595,7 +592,8 @@ mod tests {
|
||||
is_err(ev("VLOOKUP", vec![n(4.0), rf(table()), n(2.0), b(false)]), CellError::NA);
|
||||
is_err(ev("VLOOKUP", vec![n(4.0), rf(table()), n(2.0), empty()]), CellError::NA);
|
||||
close(ev("VLOOKUP", vec![n(3.0), rf(table()), n(3.0), b(false)]), 30.0);
|
||||
close(ev("VLOOKUP", vec![n(5.0), rf(table()), n(3.0), b(false)]), 0.0);
|
||||
// An empty target stays blank (the cell displays 0 only as a final result) (#374).
|
||||
assert_eq!(ev("VLOOKUP", vec![n(5.0), rf(table()), n(3.0), b(false)]), Value::Empty);
|
||||
is_err(ev("VLOOKUP", vec![n(3.0), rf(table()), n(4.0)]), CellError::Ref);
|
||||
is_err(ev("VLOOKUP", vec![n(3.0), rf(table()), n(0.0)]), CellError::Value);
|
||||
close(ev("VLOOKUP", vec![t("b*"), rf(names()), n(2.0), b(false)]), 2.0);
|
||||
|
||||
@@ -51,7 +51,7 @@ pub(crate) fn arg(args: &[Arg], i: usize) -> R<&Arg> {
|
||||
|
||||
/// Whether argument `i` was supplied (present and not an empty literal like `F(1,)`).
|
||||
pub(crate) fn has(args: &[Arg], i: usize) -> bool {
|
||||
matches!(args.get(i), Some(a) if !matches!(a.value, Value::Empty) || a.from_ref)
|
||||
matches!(args.get(i), Some(a) if !matches!(a.value, Value::Empty) || a.from_ref || a.blank_value)
|
||||
}
|
||||
|
||||
/// Number coercion for a scalar parameter: empty = 0, TRUE = 1, numeric text parses.
|
||||
|
||||
Reference in new issue
Block a user