0ct0pu5/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 2

Spreadsheet: Avoid using Value.to_string_without_side_effects()

Browse source 
We should use .to_string() and handle the possible exceptions.
This makes the displayed cell contents so much more informative than
'[object Object]' :^)
This commit is contained in:
Ali Mohammad Pur 2021-11-21 05:08:00 +03:30 • committed by Ali Mohammad Pur
parent 235eb0b1ad
commit 5f1a34bba3
Notes: sideshowbarker 2024-07-17 22:53:49 +09:00
13 changed files with 72 additions and 53 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
Userland/Applications/Spreadsheet/Cell.cpp
View file

@ -74,12 +74,12 @@ const CellType& Cell::type() const
return *CellType::get_by_name("Identity");
}
String Cell::typed_display() const
JS::ThrowCompletionOr<String> Cell::typed_display() const
{
return type().display(const_cast<Cell&>(*this), m_type_metadata);
}
JS::Value Cell::typed_js_data() const
JS::ThrowCompletionOr<JS::Value> Cell::typed_js_data() const
{
return type().js_value(const_cast<Cell&>(*this), m_type_metadata);
}

4
Userland/Applications/Spreadsheet/Cell.h
View file

@ -79,8 +79,8 @@ struct Cell : public Weakable<Cell> {
m_conditional_formats = move(fmts);
}
String typed_display() const;
JS::Value typed_js_data() const;
JS::ThrowCompletionOr<String> typed_display() const;
JS::ThrowCompletionOr<JS::Value> typed_js_data() const;
const CellType& type() const;
const CellTypeMetadata& type_metadata() const { return m_type_metadata; }

10
Userland/Applications/Spreadsheet/CellType/Date.cpp
View file

@ -21,7 +21,7 @@ DateCell::~DateCell()
{
}
String DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const
JS::ThrowCompletionOr<String> DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const
{
ScopeGuard propagate_exception { [&cell] {
if (auto exc = cell.sheet().interpreter().exception()) {
@ -29,8 +29,8 @@ String DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const
cell.set_exception(exc);
}
} };
auto timestamp = js_value(cell, metadata);
auto string = Core::DateTime::from_timestamp(TRY_OR_DISCARD(timestamp.to_i32(cell.sheet().global_object()))).to_string(metadata.format.is_empty() ? "%Y-%m-%d %H:%M:%S" : metadata.format.characters());
auto timestamp = TRY(js_value(cell, metadata));
auto string = Core::DateTime::from_timestamp(TRY(timestamp.to_i32(cell.sheet().global_object()))).to_string(metadata.format.is_empty() ? "%Y-%m-%d %H:%M:%S" : metadata.format.characters());
if (metadata.length >= 0)
return string.substring(0, metadata.length);
@ -38,10 +38,10 @@ String DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const
return string;
}
JS::Value DateCell::js_value(Cell& cell, const CellTypeMetadata&) const
JS::ThrowCompletionOr<JS::Value> DateCell::js_value(Cell& cell, const CellTypeMetadata&) const
{
auto js_data = cell.js_data();
auto value = TRY_OR_DISCARD(js_data.to_double(cell.sheet().global_object()));
auto value = TRY(js_data.to_double(cell.sheet().global_object()));
return JS::Value(value / 1000); // Turn it to seconds
}

4
Userland/Applications/Spreadsheet/CellType/Date.h
View file

@ -15,8 +15,8 @@ class DateCell : public CellType {
public:
DateCell();
virtual ~DateCell() override;
virtual String display(Cell&, const CellTypeMetadata&) const override;
virtual JS::Value js_value(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<String> display(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<JS::Value> js_value(Cell&, const CellTypeMetadata&) const override;
};
}

6
Userland/Applications/Spreadsheet/CellType/Identity.cpp
View file

@ -19,12 +19,12 @@ IdentityCell::~IdentityCell()
{
}
String IdentityCell::display(Cell& cell, const CellTypeMetadata&) const
JS::ThrowCompletionOr<String> IdentityCell::display(Cell& cell, const CellTypeMetadata&) const
{
return cell.js_data().to_string_without_side_effects();
return cell.js_data().to_string(cell.sheet().global_object());
}
JS::Value IdentityCell::js_value(Cell& cell, const CellTypeMetadata&) const
JS::ThrowCompletionOr<JS::Value> IdentityCell::js_value(Cell& cell, const CellTypeMetadata&) const
{
return cell.js_data();
}

4
Userland/Applications/Spreadsheet/CellType/Identity.h
View file

@ -15,8 +15,8 @@ class IdentityCell : public CellType {
public:
IdentityCell();
virtual ~IdentityCell() override;
virtual String display(Cell&, const CellTypeMetadata&) const override;
virtual JS::Value js_value(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<String> display(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<JS::Value> js_value(Cell&, const CellTypeMetadata&) const override;
};
}

12
Userland/Applications/Spreadsheet/CellType/Numeric.cpp
View file

@ -21,7 +21,7 @@ NumericCell::~NumericCell()
{
}
String NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const
JS::ThrowCompletionOr<String> NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const
{
ScopeGuard propagate_exception { [&cell] {
if (auto exc = cell.sheet().interpreter().exception()) {
@ -29,12 +29,12 @@ String NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const
cell.set_exception(exc);
}
} };
auto value = js_value(cell, metadata);
auto value = TRY(js_value(cell, metadata));
String string;
if (metadata.format.is_empty())
string = value.to_string_without_side_effects();
string = TRY(value.to_string(cell.sheet().global_object()));
else
string = format_double(metadata.format.characters(), TRY_OR_DISCARD(value.to_double(cell.sheet().global_object())));
string = format_double(metadata.format.characters(), TRY(value.to_double(cell.sheet().global_object())));
if (metadata.length >= 0)
return string.substring(0, metadata.length);
@ -42,7 +42,7 @@ String NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const
return string;
}
JS::Value NumericCell::js_value(Cell& cell, const CellTypeMetadata&) const
JS::ThrowCompletionOr<JS::Value> NumericCell::js_value(Cell& cell, const CellTypeMetadata&) const
{
ScopeGuard propagate_exception { [&cell] {
if (auto exc = cell.sheet().interpreter().exception()) {
@ -50,7 +50,7 @@ JS::Value NumericCell::js_value(Cell& cell, const CellTypeMetadata&) const
cell.set_exception(exc);
}
} };
return TRY_OR_DISCARD(cell.js_data().to_number(cell.sheet().global_object()));
return cell.js_data().to_number(cell.sheet().global_object());
}
}

4
Userland/Applications/Spreadsheet/CellType/Numeric.h
View file

@ -15,8 +15,8 @@ class NumericCell : public CellType {
public:
NumericCell();
virtual ~NumericCell() override;
virtual String display(Cell&, const CellTypeMetadata&) const override;
virtual JS::Value js_value(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<String> display(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<JS::Value> js_value(Cell&, const CellTypeMetadata&) const override;
};
}

8
Userland/Applications/Spreadsheet/CellType/String.cpp
View file

@ -19,18 +19,18 @@ StringCell::~StringCell()
{
}
String StringCell::display(Cell& cell, const CellTypeMetadata& metadata) const
JS::ThrowCompletionOr<String> StringCell::display(Cell& cell, const CellTypeMetadata& metadata) const
{
auto string = cell.js_data().to_string_without_side_effects();
auto string = TRY(cell.js_data().to_string(cell.sheet().global_object()));
if (metadata.length >= 0)
return string.substring(0, metadata.length);
return string;
}
JS::Value StringCell::js_value(Cell& cell, const CellTypeMetadata& metadata) const
JS::ThrowCompletionOr<JS::Value> StringCell::js_value(Cell& cell, const CellTypeMetadata& metadata) const
{
auto string = display(cell, metadata);
auto string = TRY(display(cell, metadata));
return JS::js_string(cell.sheet().interpreter().heap(), string);
}

4
Userland/Applications/Spreadsheet/CellType/String.h
View file

@ -15,8 +15,8 @@ class StringCell : public CellType {
public:
StringCell();
virtual ~StringCell() override;
virtual String display(Cell&, const CellTypeMetadata&) const override;
virtual JS::Value js_value(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<String> display(Cell&, const CellTypeMetadata&) const override;
virtual JS::ThrowCompletionOr<JS::Value> js_value(Cell&, const CellTypeMetadata&) const override;
};
}

4
Userland/Applications/Spreadsheet/CellType/Type.h
View file

@ -28,8 +28,8 @@ public:
static const CellType* get_by_name(StringView);
static Vector<StringView> names();
virtual String display(Cell&, const CellTypeMetadata&) const = 0;
virtual JS::Value js_value(Cell&, const CellTypeMetadata&) const = 0;
virtual JS::ThrowCompletionOr<String> display(Cell&, const CellTypeMetadata&) const = 0;
virtual JS::ThrowCompletionOr<JS::Value> js_value(Cell&, const CellTypeMetadata&) const = 0;
virtual ~CellType() { }
const String& name() const { return m_name; }

7
Userland/Applications/Spreadsheet/Spreadsheet.cpp
View file

@ -598,8 +598,11 @@ Vector<Vector<String>> Sheet::to_xsv() const
row.resize(column_count);
for (size_t j = 0; j < column_count; ++j) {
auto cell = at({ j, i });
if (cell)
row[j] = cell->typed_display();
if (cell) {
auto result = cell->typed_display();
if (result.has_value())
row[j] = result.value();
}
}
data.append(move(row));

54
Userland/Applications/Spreadsheet/SpreadsheetModel.cpp
View file

@ -27,29 +27,46 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role)
if (!cell)
return String::empty();
if (cell->kind() == Spreadsheet::Cell::Formula) {
if (auto exception = cell->exception()) {
StringBuilder builder;
builder.append("Error: ");
auto value = exception->value();
if (value.is_object()) {
auto& object = value.as_object();
if (is<JS::Error>(object)) {
auto error = object.get_without_side_effects("message").to_string_without_side_effects();
builder.append(error);
return builder.to_string();
}
Function<String(JS::Value)> to_string_as_exception = [&](JS::Value value) {
ScopeGuard clear_exception {
[&] {
cell->sheet().interpreter().vm().clear_exception();
}
auto error = value.to_string_without_side_effects();
// This is annoying, but whatever.
cell->sheet().interpreter().vm().clear_exception();
};
builder.append(error);
return builder.to_string();
StringBuilder builder;
builder.append("Error: "sv);
if (value.is_object()) {
auto& object = value.as_object();
if (is<JS::Error>(object)) {
auto message = object.get_without_side_effects("message");
auto error = message.to_string(cell->sheet().global_object());
if (error.is_throw_completion())
builder.append(message.to_string_without_side_effects());
else
builder.append(error.release_value());
return builder.to_string();
}
}
auto error_message = value.to_string(cell->sheet().global_object());
if (error_message.is_throw_completion())
return to_string_as_exception(error_message.release_error().value());
builder.append(error_message.release_value());
return builder.to_string();
};
if (cell->kind() == Spreadsheet::Cell::Formula) {
if (auto exception = cell->exception())
return to_string_as_exception(exception->value());
}
return cell->typed_display();
auto display = cell->typed_display();
if (display.is_error())
return to_string_as_exception(display.release_error().value());
return display.release_value();
}
if (role == GUI::ModelRole::MimeData)
@ -155,5 +172,4 @@ void SheetModel::update()
m_sheet->update();
did_update(UpdateFlag::DontInvalidateIndices);
}
}