Variables: Import Data Sets reports why a cell was refused

dataset.import replaced the reason for a refused cell with "value for
`X` must match its variable kind", so a visibility value such as
maybe and a graph value over 262,144 bytes gave the same message. The
error now keeps the line and gives the reason, and both messages have
a row in every complete-message catalog.

crates/engine/src/cmd/variablecmds/csv.rs

@@ -152,17 +152,19 @@ fn csv_escape(s: &str) -> String {
     if s.is_empty() || s.contains([',', '"', '\n', '\r']) || s != s.trim() { format!("\"{}\"", s.replace('"', "\"\"")) } else { s.to_string() }
 }
 
-/// A visibility cell: true/false, 1/0, yes/no (ASCII, case-insensitive).
-fn parse_visible_cell(s: &str, var: &str, cmd: &str) -> Result<bool> {
+/// A visibility cell: true/false, 1/0, yes/no (ASCII, case-insensitive). An error names the
+/// file line `line`.
+fn parse_visible_cell(s: &str, var: &str, line: usize, cmd: &str) -> Result<bool> {
     match s.trim().to_ascii_lowercase().as_str() {
         "true" | "1" | "yes" => Ok(true),
         "false" | "0" | "no" => Ok(false),
-        _ => Err(bad(cmd, format!("value for `{var}` must be true or false"))),
+        _ => Err(bad(cmd, format!("line {line}: value for `{var}` must be true or false"))),
     }
 }
 
-/// The value of a cell. A quoted empty text cell is an empty text value; any other empty cell is no value.
-fn cell_to_value(kind: VariableKind, cell: &str, quoted: bool, var: &str, cmd: &str) -> Result<Option<DataValue>> {
+/// The value of a cell on file line `line`. A quoted empty text cell is an empty text value; any
+/// other empty cell is no value. An error names the line and the reason.
+fn cell_to_value(kind: VariableKind, cell: &str, quoted: bool, var: &str, line: usize, cmd: &str) -> Result<Option<DataValue>> {
     // A text cell is not trimmed here (the parser trims unquoted cells); image, graph and
     // visibility cells are trimmed.
     let t = if kind == VariableKind::Text { cell } else { cell.trim() };
@@ -173,8 +175,8 @@ fn cell_to_value(kind: VariableKind, cell: &str, quoted: bool, var: &str, cmd: &
         VariableKind::Text => DataValue::Text(t.to_string()),
         VariableKind::Image => DataValue::Image(t.to_string()),
         VariableKind::Graph if t.len() <= MAX_GRAPH_CSV => DataValue::Graph(t.to_string()),
-        VariableKind::Graph => return Err(bad(cmd, format!("value for `{var}` is longer than {MAX_GRAPH_CSV} bytes"))),
-        VariableKind::Visibility => DataValue::Visible(parse_visible_cell(t, var, cmd)?),
+        VariableKind::Graph => return Err(bad(cmd, format!("line {line}: value for `{var}` is longer than {MAX_GRAPH_CSV} bytes"))),
+        VariableKind::Visibility => DataValue::Visible(parse_visible_cell(t, var, line, cmd)?),
     }))
 }
 
@@ -310,12 +312,8 @@ pub(super) fn dataset_import(s: &mut Session, p: &Value) -> Result<Value> {
         for &(col, ref var, kind) in &columns {
             let cell = row.get(col).map(String::as_str).unwrap_or("");
             let was_quoted = quoted.get(col).copied().unwrap_or(false);
-            match cell_to_value(kind, cell, was_quoted, var, C) {
-                Ok(Some(v)) => {
-                    values.insert(var.clone(), v);
-                }
-                Ok(None) => {}
-                Err(_) => return Err(bad(C, format!("line {line}: value for `{var}` must match its variable kind"))),
+            if let Some(v) = cell_to_value(kind, cell, was_quoted, var, line, C)? {
+                values.insert(var.clone(), v);
             }
         }
         parsed.push(DataSet { name, values });

crates/engine/src/tests/variables.rs

@@ -1416,6 +1416,23 @@ fn datasets_csv_row_errors_name_the_file_line() {
     assert!(long.contains("line 3:") && long.contains("longer than 255"), "{long}");
 }
 
+/// A refused cell's error gives its line and why the value was refused: a visibility value that
+/// isn't true or false, or a graph value longer than the graph data limit.
+#[test]
+fn datasets_csv_cell_errors_say_why() {
+    let mut s = session();
+    s.execute("variable.define", &json!({"name": "Show", "kind": "visibility"})).unwrap();
+    s.execute("variable.define", &json!({"name": "Sales", "kind": "graph"})).unwrap();
+    let error = |s: &mut Session, csv: &str| s.execute("dataset.import", &json!({"csv": csv})).unwrap_err();
+    let maybe = error(&mut s, "name,Show\nX,maybe");
+    let want = "line 2: value for `Show` must be true or false";
+    assert!(matches!(&maybe, EngineError::BadParams { msg, .. } if msg == want), "{maybe}");
+    let long = error(&mut s, &format!("name,Sales\nX,\"{}\"", "1,".repeat(140_000)));
+    let want = "line 2: value for `Sales` is longer than 262144 bytes";
+    assert!(matches!(&long, EngineError::BadParams { msg, .. } if msg == want), "{long}");
+    assert!(s.doc().unwrap().doc.variables.datasets.is_empty());
+}
+
 /// A header cell that names no variable defines one, unbound, in the import's undo step: `#`
 /// defines a visibility variable, `@` defines an image variable, and a cell without a prefix
 /// defines a text variable. Importing the file again finds them; a prefix of the other kind, a