diff --git a/CHANGELOG.md b/CHANGELOG.md index 6cbe68fa1..a4c143e60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Breaking**: `ValueType` gains `boolean`, `date`, `time` and `error`, and + the odf and xlsx readers report them; a boolean states 1 or 0 as its number. + `Sheet::set_cell` writes a boolean and refuses the other three. The enum + mirrors in every binding follow. + - `Text::set_style` and the edit op `setTextStyle {id, style}` write bold, italic, underline, strikethrough, highlight, colour and size onto a run of an odf, docx or pptx document. The other engines refuse it. diff --git a/apple/include/OdrCoreObjC/ODRDocumentElement.h b/apple/include/OdrCoreObjC/ODRDocumentElement.h index 3b9114a66..164828485 100644 --- a/apple/include/OdrCoreObjC/ODRDocumentElement.h +++ b/apple/include/OdrCoreObjC/ODRDocumentElement.h @@ -67,6 +67,10 @@ typedef NS_ENUM(NSInteger, ODRValueType) { ODRValueTypeUnknown = 0, ODRValueTypeString, ODRValueTypeFloatNumber, + ODRValueTypeBoolean, + ODRValueTypeDate, + ODRValueTypeTime, + ODRValueTypeError, } NS_SWIFT_NAME(ValueType); /// A node in a document's element tree — `odr::Element`. diff --git a/apple/src/ODRDocumentElement.mm b/apple/src/ODRDocumentElement.mm index 111cddd36..b2a93586e 100644 --- a/apple/src/ODRDocumentElement.mm +++ b/apple/src/ODRDocumentElement.mm @@ -59,6 +59,10 @@ ODR_SAME_ENUM(ODRValueTypeUnknown, odr::ValueType::unknown); ODR_SAME_ENUM(ODRValueTypeString, odr::ValueType::string); ODR_SAME_ENUM(ODRValueTypeFloatNumber, odr::ValueType::float_number); +ODR_SAME_ENUM(ODRValueTypeBoolean, odr::ValueType::boolean); +ODR_SAME_ENUM(ODRValueTypeDate, odr::ValueType::date); +ODR_SAME_ENUM(ODRValueTypeTime, odr::ValueType::time); +ODR_SAME_ENUM(ODRValueTypeError, odr::ValueType::error); namespace { diff --git a/docs/design/spreadsheet-editing.md b/docs/design/spreadsheet-editing.md index 9258c857c..f96f8b4e9 100644 --- a/docs/design/spreadsheet-editing.md +++ b/docs/design/spreadsheet-editing.md @@ -45,7 +45,7 @@ results go stale the moment an input changes. | XLSX edit | `sheet_set_cell` | Writes a cell value (step 0.2, landed); `text_set_content` is still a no-op | | XLSX save | `ooxml_spreadsheet_document.cpp::save` | Writes back the worksheets and `workbook.xml`, copies the rest (step 0.2, landed) | | XLSX cells | `Sheet.cells` `(col,row) → {node, id}` map | Off-tree; a position the file states no `` for is written by `insert_cell` (step 2.2, landed) | -| Cell value | `SheetCellAdapter` | `sheet_cell_value` reads the number and the formula (step 0.1, landed); `sheet_cell_value_type` stays the cheap question the renderer asks. Dates, booleans and errors still report `string` | +| Cell value | `SheetCellAdapter` | `sheet_cell_value` reads the number and the formula (step 0.1, landed); `sheet_cell_value_type` stays the cheap question the renderer asks. A boolean, a date, a time and an error report their own kind; an xlsx date is a `float_number` serial until number formats are read | | Number formats | — | Not parsed in either engine. ODS shows the producer's cached `text:p`; XLSX shows the raw `` (a date is its serial) | | Formulas | `sheet_cell_value` | The expression is read and handed out as a string (step 0.1, landed); nothing parses or evaluates it. XLSX shows the cached ``, ODS the cached `text:p`. `xls` and `numbers` drop the expression at parse time | | Browser: sheet script | `html/frontend/spreadsheet.js` | Hover/pin, raise a clipped cell over its neighbours, sort rows in the DOM. Sorting reorders ``s, so a row's identity is its `` label, not its index. Publishes `odr.sheet` (step 1.1, landed), and the value and reflow half of it (steps 1.2/1.3, landed) | diff --git a/jni/java/app/opendocument/core/ValueType.java b/jni/java/app/opendocument/core/ValueType.java index 63efa42c8..ae89cff87 100644 --- a/jni/java/app/opendocument/core/ValueType.java +++ b/jni/java/app/opendocument/core/ValueType.java @@ -2,7 +2,7 @@ /** Mirrors {@code odr::ValueType}; constant order must match the C++ declaration. */ public enum ValueType { - UNKNOWN, STRING, FLOAT_NUMBER; + UNKNOWN, STRING, FLOAT_NUMBER, BOOLEAN, DATE, TIME, ERROR; static ValueType fromNative(int code) { return code < 0 ? null : values()[code]; diff --git a/python/src/bind_document.cpp b/python/src/bind_document.cpp index 828ad8f04..de97e556a 100644 --- a/python/src/bind_document.cpp +++ b/python/src/bind_document.cpp @@ -92,7 +92,11 @@ void odr_python::bind_document(py::module_ &m) { py::enum_(m, "ValueType") .value("unknown", odr::ValueType::unknown) .value("string", odr::ValueType::string) - .value("float_number", odr::ValueType::float_number); + .value("float_number", odr::ValueType::float_number) + .value("boolean", odr::ValueType::boolean) + .value("date", odr::ValueType::date) + .value("time", odr::ValueType::time) + .value("error", odr::ValueType::error); py::class_(m, "TableDimensions") .def(py::init<>()) diff --git a/src/odr/document_element.hpp b/src/odr/document_element.hpp index 3a603d255..bdbc78450 100644 --- a/src/odr/document_element.hpp +++ b/src/odr/document_element.hpp @@ -131,11 +131,17 @@ enum class AnchorType { at_paragraph, }; -/// Collection of value types. +/// What a cell states, as the file types it. A percentage and a currency are +/// a `string` stating a number until number formats are read, and so is an +/// ooxml date, which is a serial its format shows. enum class ValueType { unknown, string, float_number, + boolean, ///< states 1 or 0 as its number + date, + time, + error, ///< an evaluation error the file recorded }; /// @brief What a sheet cell holds: what @ref SheetCell::value reads out of one, diff --git a/src/odr/internal/odf/odf_document.cpp b/src/odr/internal/odf/odf_document.cpp index f4b683b58..2fb230a89 100644 --- a/src/odr/internal/odf/odf_document.cpp +++ b/src/odr/internal/odf/odf_document.cpp @@ -187,6 +187,41 @@ void remove_value_attributes(pugi::xml_node node) { } } +/// [ODF 1.2] 19.385 `office:value-type`. A percentage and a currency stay a +/// string until number formats are read, since only their text shows them. +ValueType value_type_of(const pugi::xml_node node) { + const char *value_type = node.attribute("office:value-type").value(); + if (std::strcmp("float", value_type) == 0) { + return ValueType::float_number; + } + if (std::strcmp("boolean", value_type) == 0) { + return ValueType::boolean; + } + if (std::strcmp("date", value_type) == 0) { + return ValueType::date; + } + if (std::strcmp("time", value_type) == 0) { + return ValueType::time; + } + return ValueType::string; +} + +/// Whether the engine has a form to write @p value in. +bool writable(const CellValue &value) { + switch (value.type()) { + case ValueType::unknown: + case ValueType::string: + case ValueType::float_number: + case ValueType::boolean: + return true; + case ValueType::date: + case ValueType::time: + case ValueType::error: + return false; + } + return false; +} + using TreeEditor = xml::TreeEditor; using xml::NodeSpan; @@ -456,6 +491,9 @@ class ElementAdapter final : public AdapterBase { void sheet_set_cell(const ElementIdentifier element_id, const std::uint32_t column, const std::uint32_t row, const CellValue &value) const override { + if (!writable(value)) { + throw UnsupportedOperation(); + } const ElementRegistry::Sheet::Cell *cell = m_registry->sheet_element_at(element_id).cell(column, row); @@ -497,6 +535,16 @@ class ElementAdapter final : public AdapterBase { node.append_attribute("office:value") .set_value(fmt::format("{}", value.number()).c_str()); break; + case ValueType::boolean: + node.append_attribute("office:value-type").set_value("boolean"); + node.append_attribute("office:boolean-value") + .set_value(value.has_number() && value.number() != 0 ? "true" + : "false"); + break; + case ValueType::date: + case ValueType::time: + case ValueType::error: + throw UnsupportedOperation(); // `writable` refused these above } drop_stale_results(element_id, column, row); @@ -632,15 +680,10 @@ class ElementAdapter final : public AdapterBase { } [[nodiscard]] ValueType sheet_cell_value_type(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const char *value_type = node.attribute("office:value-type").value(); - std::strcmp("float", value_type) == 0) { - return ValueType::float_number; - } - return ValueType::string; + return value_type_of(get_node(element_id)); } - /// [ODF 1.2] 19.386 `office:value`, 19.642 `table:formula`. A date, a time - /// and a boolean state their value elsewhere and are read as their text. + /// [ODF 1.2] 19.386 `office:value`, 19.642 `table:formula`. A date and a + /// time state their value as text; a boolean states 1 or 0. [[nodiscard]] CellValue sheet_cell_value(const ElementIdentifier element_id) const override { const pugi::xml_node node = get_node(element_id); @@ -650,6 +693,12 @@ class ElementAdapter final : public AdapterBase { util::number::parse(node.attribute("office:value").value())) { result = result.with_number(*number); } + if (result.type() == ValueType::boolean) { + const bool set = + std::strcmp("true", node.attribute("office:boolean-value").value()) == + 0; + result = result.with_number(set ? 1 : 0); + } if (const pugi::xml_attribute formula = node.attribute("table:formula")) { result = result.with_formula(formula.value()); } @@ -893,12 +942,7 @@ class ElementAdapter final : public AdapterBase { } [[nodiscard]] ValueType table_cell_value_type(const ElementIdentifier element_id) const override { - const pugi::xml_node node = get_node(element_id); - if (const char *value_type = node.attribute("office:value-type").value(); - std::strcmp("float", value_type) == 0) { - return ValueType::float_number; - } - return ValueType::string; + return value_type_of(get_node(element_id)); } [[nodiscard]] TableCellStyle table_cell_style(const ElementIdentifier element_id) const override { diff --git a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp index 0296dadd8..6731b5e95 100644 --- a/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp +++ b/src/odr/internal/ooxml/spreadsheet/ooxml_spreadsheet_document.cpp @@ -236,6 +236,10 @@ class ElementAdapter final : public AdapterBase { void sheet_set_cell(const ElementIdentifier element_id, const std::uint32_t column, const std::uint32_t row, const CellValue &value) const override { + if (value.type() == ValueType::date || value.type() == ValueType::time || + value.type() == ValueType::error) { + throw UnsupportedOperation(); // no form to write them in yet + } const ElementRegistry::Sheet &sheet = m_registry->sheet_element_at(element_id); const ElementRegistry::Sheet::Cell *cell = sheet.cell(column, row); @@ -288,6 +292,19 @@ class ElementAdapter final : public AdapterBase { m_registry->create_text_element(value_node, value_node); m_registry->append_child(cell_id, text_id); } break; + case ValueType::boolean: { + node.append_attribute("t").set_value("b"); + const pugi::xml_node value_node = node.append_child("v"); + value_node.text().set(value.has_number() && value.number() != 0 ? "1" + : "0"); + const auto &[text_id, unused1, unused2] = + m_registry->create_text_element(value_node, value_node); + m_registry->append_child(cell_id, text_id); + } break; + case ValueType::date: + case ValueType::time: + case ValueType::error: + throw UnsupportedOperation(); // refused above } } @@ -358,8 +375,16 @@ class ElementAdapter final : public AdapterBase { // inline ("inlineStr"), or formula ("str") cells. const pugi::xml_node node = get_node(element_id); const std::string type = node.attribute("t").value(); - if (type == "s" || type == "str" || type == "inlineStr" || type == "b" || - type == "e" || type == "d") { + if (type == "b") { + return ValueType::boolean; + } + if (type == "e") { + return ValueType::error; + } + if (type == "d") { + return ValueType::date; + } + if (type == "s" || type == "str" || type == "inlineStr") { return ValueType::string; } if (node.child("v")) { @@ -374,7 +399,8 @@ class ElementAdapter final : public AdapterBase { const pugi::xml_node node = get_node(element_id); CellValue result = CellValue(sheet_cell_value_type(element_id)); - if (result.type() == ValueType::float_number) { + if (result.type() == ValueType::float_number || + result.type() == ValueType::boolean) { if (const std::optional number = util::number::parse(node.child("v").text().get())) { result = result.with_number(*number); diff --git a/test/src/enum_ordinals_test.cpp b/test/src/enum_ordinals_test.cpp index a569fc899..5357ea4a4 100644 --- a/test/src/enum_ordinals_test.cpp +++ b/test/src/enum_ordinals_test.cpp @@ -203,6 +203,10 @@ TEST(EnumOrdinals, value_type) { EXPECT_EQ(ordinal(ValueType::unknown), 0); EXPECT_EQ(ordinal(ValueType::string), 1); EXPECT_EQ(ordinal(ValueType::float_number), 2); + EXPECT_EQ(ordinal(ValueType::boolean), 3); + EXPECT_EQ(ordinal(ValueType::date), 4); + EXPECT_EQ(ordinal(ValueType::time), 5); + EXPECT_EQ(ordinal(ValueType::error), 6); } TEST(EnumOrdinals, list_type) { diff --git a/test/src/internal/odf/odf_sheet_value_test.cpp b/test/src/internal/odf/odf_sheet_value_test.cpp index a5ca93eee..29e20f26a 100644 --- a/test/src/internal/odf/odf_sheet_value_test.cpp +++ b/test/src/internal/odf/odf_sheet_value_test.cpp @@ -98,3 +98,29 @@ TEST(OdfSheetValue, a_number_is_read_in_one_spelling_only) { EXPECT_FALSE(value.has_number()); } + +/// [ODF 1.2] 19.385: a boolean states `office:boolean-value`, read as 1 or 0. +TEST(OdfSheetValue, a_boolean_cell_is_typed_and_states_one_or_zero) { + const CellValue value = value_of( + R"()" + R"(TRUE)"); + + EXPECT_EQ(value.type(), ValueType::boolean); + ASSERT_TRUE(value.has_number()); + EXPECT_DOUBLE_EQ(value.number(), 1); +} + +/// A date and a time state their value as text, which is what the type says. +TEST(OdfSheetValue, a_date_and_a_time_cell_are_typed_and_state_no_number) { + const CellValue date = value_of( + R"()" + R"(31.01.2024)"); + EXPECT_EQ(date.type(), ValueType::date); + EXPECT_FALSE(date.has_number()); + + const CellValue time = value_of( + R"()" + R"(10:30)"); + EXPECT_EQ(time.type(), ValueType::time); + EXPECT_FALSE(time.has_number()); +} diff --git a/test/src/internal/odf/odf_sheet_write_test.cpp b/test/src/internal/odf/odf_sheet_write_test.cpp index aee195ed1..1fc4178ce 100644 --- a/test/src/internal/odf/odf_sheet_write_test.cpp +++ b/test/src/internal/odf/odf_sheet_write_test.cpp @@ -621,3 +621,30 @@ TEST(OdfSheetWrite, a_number_stating_none_leaves_the_cell_alone) { ValueNotStated); EXPECT_EQ(sheet.cell(0, 0).value().text(), "old"); } + +TEST(OdfSheetWrite, a_boolean_is_written_as_its_value_and_text) { + const Document document = document_of(flat_sheet(string_cell("old"))); + const Sheet sheet = first_sheet(document); + + sheet.set_cell( + 0, 0, CellValue(ValueType::boolean).with_number(1).with_text("TRUE")); + + const CellValue value = sheet.cell(0, 0).value(); + EXPECT_EQ(value.type(), ValueType::boolean); + ASSERT_TRUE(value.has_number()); + EXPECT_DOUBLE_EQ(value.number(), 1); + EXPECT_EQ(value.text(), "TRUE"); +} + +/// No form to write these in yet, and the refusal leaves the cell as it was. +TEST(OdfSheetWrite, a_date_a_time_and_an_error_refuse_to_be_written) { + const Document document = document_of(flat_sheet(string_cell("old"))); + const Sheet sheet = first_sheet(document); + + for (const ValueType type : + {ValueType::date, ValueType::time, ValueType::error}) { + EXPECT_THROW(sheet.set_cell(0, 0, CellValue(type).with_text("x")), + UnsupportedOperation); + } + EXPECT_EQ(sheet.cell(0, 0).value().text(), "old"); +} diff --git a/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp b/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp index 6e30867e7..d532083ec 100644 --- a/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp +++ b/test/src/internal/ooxml/ooxml_spreadsheet_value_test.cpp @@ -116,11 +116,26 @@ TEST(OoxmlSpreadsheetValue, a_shared_formula_that_does_not_parse_is_kept) { EXPECT_EQ(formula_at(data, 2, 1), "A1 +"); } -/// `` holds `1`, not a quantity, and `c/@t="b"` types the cell a string. -TEST(OoxmlSpreadsheetValue, a_boolean_cell_states_no_number) { +/// ECMA-376 18.18.11: `c/@t="b"` types the cell a boolean, `` its 1 or 0. +TEST(OoxmlSpreadsheetValue, a_boolean_cell_is_typed_and_states_one_or_zero) { const CellValue value = value_of(R"(1)"); - EXPECT_EQ(value.type(), ValueType::string); - EXPECT_FALSE(value.has_number()); + EXPECT_EQ(value.type(), ValueType::boolean); + ASSERT_TRUE(value.has_number()); + EXPECT_DOUBLE_EQ(value.number(), 1); +} + +/// An error cell states its text and a date cell its ISO 8601 text; neither +/// is a number. +TEST(OoxmlSpreadsheetValue, an_error_and_a_date_cell_are_typed) { + const CellValue error = + value_of(R"(#DIV/0!)"); + EXPECT_EQ(error.type(), ValueType::error); + EXPECT_FALSE(error.has_number()); + + const CellValue date = + value_of(R"(2024-01-31)"); + EXPECT_EQ(date.type(), ValueType::date); + EXPECT_FALSE(date.has_number()); } diff --git a/test/src/internal/ooxml/ooxml_spreadsheet_write_test.cpp b/test/src/internal/ooxml/ooxml_spreadsheet_write_test.cpp index 759a84c7c..6a82571e8 100644 --- a/test/src/internal/ooxml/ooxml_spreadsheet_write_test.cpp +++ b/test/src/internal/ooxml/ooxml_spreadsheet_write_test.cpp @@ -103,6 +103,35 @@ TEST(OoxmlSpreadsheetWrite, a_cleared_cell_states_nothing) { EXPECT_EQ(sheet.cell(0, 0).value().text(), ""); } +TEST(OoxmlSpreadsheetWrite, a_boolean_lands_as_a_boolean) { + const Document document = + decode(workbook(R"(1)")); + const Sheet sheet = first_sheet(document); + + sheet.set_cell(0, 0, CellValue(ValueType::boolean).with_number(0)); + + const CellValue value = sheet.cell(0, 0).value(); + EXPECT_EQ(value.type(), ValueType::boolean); + ASSERT_TRUE(value.has_number()); + EXPECT_DOUBLE_EQ(value.number(), 0); + EXPECT_NE(worksheet_of(document).find(R"(0)"), + std::string::npos); +} + +/// No form to write these in yet, and the refusal leaves the cell as it was. +TEST(OoxmlSpreadsheetWrite, a_date_a_time_and_an_error_refuse_to_be_written) { + const Document document = + decode(workbook(R"(1)")); + const Sheet sheet = first_sheet(document); + + for (const ValueType type : + {ValueType::date, ValueType::time, ValueType::error}) { + EXPECT_THROW(sheet.set_cell(0, 0, CellValue(type).with_text("x")), + UnsupportedOperation); + } + EXPECT_DOUBLE_EQ(sheet.cell(0, 0).value().number(), 1); +} + TEST(OoxmlSpreadsheetWrite, a_formula_cell_refuses_to_be_written) { const Document document = decode( workbook(R"(SUM(B1:C1)7)"));