Parcourir la source

Spreadsheet: Do not assume that a worksheet always exists

Fixes #5910.
Fixes #4822 (as far as I can tell).
AnotherTest il y a 4 ans
Parent
commit
ba2df70eab

+ 1 - 2
Userland/Applications/Spreadsheet/SpreadsheetView.h

@@ -108,8 +108,7 @@ class SpreadsheetView final : public GUI::Widget {
 public:
     ~SpreadsheetView();
 
-    const Sheet& sheet() const { return *m_sheet; }
-    Sheet& sheet() { return *m_sheet; }
+    Sheet* sheet_if_available() { return m_sheet; }
 
     const GUI::ModelIndex* cursor() const
     {

+ 31 - 12
Userland/Applications/Spreadsheet/SpreadsheetWidget.cpp

@@ -59,10 +59,14 @@ SpreadsheetWidget::SpreadsheetWidget(NonnullRefPtrVector<Sheet>&& sheets, bool s
     auto& help_button = top_bar.add<GUI::Button>("🛈");
     help_button.set_fixed_size(20, 20);
     help_button.on_click = [&](auto) {
-        auto docs = m_selected_view->sheet().gather_documentation();
-        auto help_window = HelpWindow::the(window());
-        help_window->set_docs(move(docs));
-        help_window->show();
+        if (!m_selected_view) {
+            GUI::MessageBox::show_error(window(), "Can only show function documentation/help when a worksheet exists and is open");
+        } else if (auto* sheet_ptr = m_selected_view->sheet_if_available()) {
+            auto docs = sheet_ptr->gather_documentation();
+            auto help_window = HelpWindow::the(window());
+            help_window->set_docs(move(docs));
+            help_window->show();
+        }
     };
 
     auto& cell_value_editor = top_bar.add<GUI::TextEditor>(GUI::TextEditor::Type::SingleLine);
@@ -98,7 +102,9 @@ SpreadsheetWidget::SpreadsheetWidget(NonnullRefPtrVector<Sheet>&& sheets, bool s
     auto rename_action = GUI::Action::create("Rename...", [this](auto&) {
         VERIFY(m_tab_context_menu_sheet_view);
 
-        auto& sheet = m_tab_context_menu_sheet_view->sheet();
+        auto* sheet_ptr = m_tab_context_menu_sheet_view->sheet_if_available();
+        VERIFY(sheet_ptr); // How did we get here without a sheet?
+        auto& sheet = *sheet_ptr;
         String new_name;
         if (GUI::InputBox::show(window(), new_name, String::formatted("New name for '{}'", sheet.name()), "Rename sheet") == GUI::Dialog::ExecOK) {
             sheet.set_name(new_name);
@@ -139,9 +145,13 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
         if (m_selected_view) {
             m_selected_view->on_selection_changed = nullptr;
             m_selected_view->on_selection_dropped = nullptr;
-        };
+        }
         m_selected_view = &static_cast<SpreadsheetView&>(selected_widget);
         m_selected_view->on_selection_changed = [&](Vector<Position>&& selection) {
+            auto* sheet_ptr = m_selected_view->sheet_if_available();
+            // How did this even happen?
+            VERIFY(sheet_ptr);
+            auto& sheet = *sheet_ptr;
             if (selection.is_empty()) {
                 m_current_cell_label->set_enabled(false);
                 m_current_cell_label->set_text({});
@@ -156,9 +166,9 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
             if (selection.size() == 1) {
                 auto& position = selection.first();
                 m_current_cell_label->set_enabled(true);
-                m_current_cell_label->set_text(position.to_cell_identifier(m_selected_view->sheet()));
+                m_current_cell_label->set_text(position.to_cell_identifier(sheet));
 
-                auto& cell = m_selected_view->sheet().ensure(position);
+                auto& cell = sheet.ensure(position);
                 m_cell_value_editor->on_change = nullptr;
                 m_cell_value_editor->set_text(cell.source());
                 m_cell_value_editor->on_change = [&] {
@@ -167,7 +177,7 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
                     auto offset = m_cell_value_editor->cursor().column();
                     try_generate_tip_for_input_expression(text, offset);
                     cell.set_data(move(text));
-                    m_selected_view->sheet().update();
+                    sheet.update();
                     update();
                 };
                 m_cell_value_editor->set_enabled(true);
@@ -183,7 +193,7 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
 
             Vector<Cell*> cells;
             for (auto& position : selection)
-                cells.append(&m_selected_view->sheet().ensure(position));
+                cells.append(&sheet.ensure(position));
 
             auto first_cell = cells.first();
             m_cell_value_editor->on_change = nullptr;
@@ -193,13 +203,17 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
             m_cell_value_editor->on_focusout = [this] { m_should_change_selected_cells = false; };
             m_cell_value_editor->on_change = [cells = move(cells), this] {
                 if (m_should_change_selected_cells) {
+                    auto* sheet_ptr = m_selected_view->sheet_if_available();
+                    if (!sheet_ptr)
+                        return;
+                    auto& sheet = *sheet_ptr;
                     auto text = m_cell_value_editor->text();
                     // FIXME: Lines?
                     auto offset = m_cell_value_editor->cursor().column();
                     try_generate_tip_for_input_expression(text, offset);
                     for (auto* cell : cells)
                         cell->set_data(text);
-                    m_selected_view->sheet().update();
+                    sheet.update();
                     update();
                 }
             };
@@ -230,6 +244,12 @@ void SpreadsheetWidget::setup_tabs(NonnullRefPtrVector<Sheet> new_sheets)
 
 void SpreadsheetWidget::try_generate_tip_for_input_expression(StringView source, size_t cursor_offset)
 {
+    auto* sheet_ptr = m_selected_view->sheet_if_available();
+    if (!sheet_ptr)
+        return;
+
+    auto& sheet = *sheet_ptr;
+
     m_inline_documentation_window->set_rect(m_cell_value_editor->screen_relative_rect().translated(0, m_cell_value_editor->height() + 7).inflated(6, 6));
     if (!m_selected_view || !source.starts_with('=')) {
         m_inline_documentation_window->hide();
@@ -242,7 +262,6 @@ void SpreadsheetWidget::try_generate_tip_for_input_expression(StringView source,
     }
 
     auto& [name, index] = maybe_function_and_argument.value();
-    auto& sheet = m_selected_view->sheet();
     auto text = sheet.generate_inline_documentation_for(name, index);
     if (text.is_empty()) {
         m_inline_documentation_window->hide();

+ 1 - 2
Userland/Applications/Spreadsheet/SpreadsheetWidget.h

@@ -46,8 +46,7 @@ public:
     void add_sheet(NonnullRefPtr<Sheet>&&);
 
     const String& current_filename() const { return m_workbook->current_filename(); }
-    const Sheet& current_worksheet() const { return m_selected_view->sheet(); }
-    Sheet& current_worksheet() { return m_selected_view->sheet(); }
+    Sheet* current_worksheet_if_available() { return m_selected_view ? m_selected_view->sheet_if_available() : nullptr; }
     void set_filename(const String& filename);
 
     Workbook& workbook() { return *m_workbook; }

+ 30 - 14
Userland/Applications/Spreadsheet/main.cpp

@@ -36,6 +36,7 @@
 #include <LibGUI/Icon.h>
 #include <LibGUI/Menu.h>
 #include <LibGUI/MenuBar.h>
+#include <LibGUI/MessageBox.h>
 #include <LibGUI/Window.h>
 #include <unistd.h>
 
@@ -169,7 +170,13 @@ int main(int argc, char* argv[])
         /// - action: copy/cut
         /// - currently selected cell
         /// - selected cell+
-        auto& cells = spreadsheet_widget.current_worksheet().selected_cells();
+        auto* worksheet_ptr = spreadsheet_widget.current_worksheet_if_available();
+        if (!worksheet_ptr) {
+            GUI::MessageBox::show_error(window, "There are no active worksheets");
+            return;
+        }
+        auto& worksheet = *worksheet_ptr;
+        auto& cells = worksheet.selected_cells();
         VERIFY(!cells.is_empty());
         StringBuilder text_builder, url_builder;
         url_builder.append(is_cut ? "cut\n" : "copy\n");
@@ -177,20 +184,20 @@ int main(int argc, char* argv[])
         auto cursor = spreadsheet_widget.current_selection_cursor();
         if (cursor) {
             Spreadsheet::Position position { (size_t)cursor->column(), (size_t)cursor->row() };
-            url_builder.append(position.to_url(spreadsheet_widget.current_worksheet()).to_string());
+            url_builder.append(position.to_url(worksheet).to_string());
             url_builder.append('\n');
         }
 
         for (auto& cell : cells) {
             if (first && !cursor) {
-                url_builder.append(cell.to_url(spreadsheet_widget.current_worksheet()).to_string());
+                url_builder.append(cell.to_url(worksheet).to_string());
                 url_builder.append('\n');
             }
 
-            url_builder.append(cell.to_url(spreadsheet_widget.current_worksheet()).to_string());
+            url_builder.append(cell.to_url(worksheet).to_string());
             url_builder.append('\n');
 
-            auto cell_data = spreadsheet_widget.current_worksheet().at(cell);
+            auto cell_data = worksheet.at(cell);
             if (!first)
                 text_builder.append('\t');
             if (cell_data)
@@ -209,12 +216,17 @@ int main(int argc, char* argv[])
     edit_menu.add_action(GUI::CommonActions::make_paste_action([&](auto&) {
         ScopeGuard update_after_paste { [&] { spreadsheet_widget.update(); } };
 
-        auto& cells = spreadsheet_widget.current_worksheet().selected_cells();
+        auto* worksheet_ptr = spreadsheet_widget.current_worksheet_if_available();
+        if (!worksheet_ptr) {
+            GUI::MessageBox::show_error(window, "There are no active worksheets");
+            return;
+        }
+        auto& sheet = *worksheet_ptr;
+        auto& cells = sheet.selected_cells();
         VERIFY(!cells.is_empty());
         const auto& data = GUI::Clipboard::the().data_and_type();
         if (auto spreadsheet_data = data.metadata.get("text/x-spreadsheet-data"); spreadsheet_data.has_value()) {
             Vector<Spreadsheet::Position> source_positions, target_positions;
-            auto& sheet = spreadsheet_widget.current_worksheet();
             auto lines = spreadsheet_data.value().split_view('\n');
             auto action = lines.take_first();
 
@@ -225,7 +237,7 @@ int main(int argc, char* argv[])
                     source_positions.append(position.release_value());
             }
 
-            for (auto& position : spreadsheet_widget.current_worksheet().selected_cells())
+            for (auto& position : sheet.selected_cells())
                 target_positions.append(position);
 
             if (source_positions.is_empty())
@@ -234,8 +246,8 @@ int main(int argc, char* argv[])
             auto first_position = source_positions.take_first();
             sheet.copy_cells(move(source_positions), move(target_positions), first_position, action == "cut" ? Spreadsheet::Sheet::CopyOperation::Cut : Spreadsheet::Sheet::CopyOperation::Copy);
         } else {
-            for (auto& cell : spreadsheet_widget.current_worksheet().selected_cells())
-                spreadsheet_widget.current_worksheet().ensure(cell).set_data(StringView { data.data.data(), data.data.size() });
+            for (auto& cell : sheet.selected_cells())
+                sheet.ensure(cell).set_data(StringView { data.data.data(), data.data.size() });
             spreadsheet_widget.update();
         }
     },
@@ -245,10 +257,14 @@ int main(int argc, char* argv[])
 
     help_menu.add_action(GUI::Action::create(
         "Functions Help", [&](auto&) {
-            auto docs = spreadsheet_widget.current_worksheet().gather_documentation();
-            auto help_window = Spreadsheet::HelpWindow::the(window);
-            help_window->set_docs(move(docs));
-            help_window->show();
+            if (auto* worksheet_ptr = spreadsheet_widget.current_worksheet_if_available()) {
+                auto docs = worksheet_ptr->gather_documentation();
+                auto help_window = Spreadsheet::HelpWindow::the(window);
+                help_window->set_docs(move(docs));
+                help_window->show();
+            } else {
+                GUI::MessageBox::show_error(window, "Cannot prepare documentation/help without an active worksheet");
+            }
         },
         window));