From 102065a8a9e0aa2a11821931f7f497c4afae9ec1 Mon Sep 17 00:00:00 2001 From: AnotherTest Date: Thu, 25 Mar 2021 10:25:24 +0430 Subject: [PATCH] Spreadsheet: Show the error (if any) in csv import dialog's preview ...and don't try to read from a CSV that has errors. Fixes #5942. --- .../Applications/Spreadsheet/ExportDialog.cpp | 7 +----- .../Applications/Spreadsheet/ExportDialog.h | 6 +++++ .../Applications/Spreadsheet/ImportDialog.cpp | 23 ++++++++++++++----- .../Applications/Spreadsheet/ImportDialog.h | 7 ++++++ .../Applications/Spreadsheet/Readers/XSV.cpp | 2 ++ .../Applications/Spreadsheet/csv_import.gml | 12 ++++++++-- 6 files changed, 43 insertions(+), 14 deletions(-) diff --git a/Userland/Applications/Spreadsheet/ExportDialog.cpp b/Userland/Applications/Spreadsheet/ExportDialog.cpp index f911558396..dd922020d7 100644 --- a/Userland/Applications/Spreadsheet/ExportDialog.cpp +++ b/Userland/Applications/Spreadsheet/ExportDialog.cpp @@ -97,12 +97,7 @@ CSVExportDialogPage::CSVExportDialogPage(const Sheet& sheet) m_data_preview_text_editor->set_should_hide_unnecessary_scrollbars(true); - Vector quote_escape_items { - // Note: Keep in sync with Writer::WriterTraits::QuoteEscape. - "Repeat", - "Backslash", - }; - m_quote_escape_combo_box->set_model(GUI::ItemListModel::create(quote_escape_items)); + m_quote_escape_combo_box->set_model(GUI::ItemListModel::create(m_quote_escape_items)); // By default, use commas, double quotes with repeat, disable headers, and quote only the fields that require quoting. m_delimiter_comma_radio->set_checked(true); diff --git a/Userland/Applications/Spreadsheet/ExportDialog.h b/Userland/Applications/Spreadsheet/ExportDialog.h index d1bb80816d..9cdbfbe27a 100644 --- a/Userland/Applications/Spreadsheet/ExportDialog.h +++ b/Userland/Applications/Spreadsheet/ExportDialog.h @@ -69,6 +69,12 @@ private: RefPtr m_export_header_check_box; RefPtr m_quote_all_fields_check_box; RefPtr m_data_preview_text_editor; + Vector m_quote_escape_items { + // Note: Keep in sync with Writer::WriterTraits::QuoteEscape. + "Repeat", + "Backslash", + }; + String m_temp_output_file_path; }; diff --git a/Userland/Applications/Spreadsheet/ImportDialog.cpp b/Userland/Applications/Spreadsheet/ImportDialog.cpp index 39400e41a6..d607a1e4d8 100644 --- a/Userland/Applications/Spreadsheet/ImportDialog.cpp +++ b/Userland/Applications/Spreadsheet/ImportDialog.cpp @@ -38,6 +38,7 @@ #include #include #include +#include #include #include #include @@ -71,13 +72,10 @@ CSVImportDialogPage::CSVImportDialogPage(StringView csv) m_trim_leading_field_spaces_check_box = m_page->body_widget().find_descendant_of_type_named("trim_leading_field_spaces_check_box"); m_trim_trailing_field_spaces_check_box = m_page->body_widget().find_descendant_of_type_named("trim_trailing_field_spaces_check_box"); m_data_preview_table_view = m_page->body_widget().find_descendant_of_type_named("data_preview_table_view"); + m_data_preview_error_label = m_page->body_widget().find_descendant_of_type_named("data_preview_error_label"); + m_data_preview_widget = m_page->body_widget().find_descendant_of_type_named("data_preview_widget"); - Vector quote_escape_items { - // Note: Keep in sync with Reader::ParserTraits::QuoteEscape. - "Repeat", - "Backslash", - }; - m_quote_escape_combo_box->set_model(GUI::ItemListModel::create(quote_escape_items)); + m_quote_escape_combo_box->set_model(GUI::ItemListModel::create(m_quote_escape_items)); // By default, use commas, double quotes with repeat, and disable headers. m_delimiter_comma_radio->set_checked(true); @@ -178,14 +176,24 @@ void CSVImportDialogPage::update_preview() m_previously_made_reader = make_reader(); if (!m_previously_made_reader.has_value()) { m_data_preview_table_view->set_model(nullptr); + m_data_preview_error_label->set_text("Could not read the given file"); + m_data_preview_widget->set_active_widget(m_data_preview_error_label); return; } auto& reader = *m_previously_made_reader; + if (reader.has_error()) { + m_data_preview_table_view->set_model(nullptr); + m_data_preview_error_label->set_text(String::formatted("XSV parse error:\n{}", reader.error_string())); + m_data_preview_widget->set_active_widget(m_data_preview_error_label); + return; + } + auto headers = reader.headers(); m_data_preview_table_view->set_model( GUI::ItemListModel>::create(reader, headers, min(8ul, reader.size()))); + m_data_preview_widget->set_active_widget(m_data_preview_table_view); m_data_preview_table_view->update(); } @@ -207,6 +215,9 @@ Result, String> ImportDialog::make_and_run_for(String NonnullRefPtrVector sheets; if (reader.has_value()) { + if (reader.value().has_error()) + return String::formatted("CSV Import failed: {}", reader.value().error_string()); + auto sheet = Sheet::from_xsv(reader.value(), workbook); if (sheet) sheets.append(sheet.release_nonnull()); diff --git a/Userland/Applications/Spreadsheet/ImportDialog.h b/Userland/Applications/Spreadsheet/ImportDialog.h index 8fbc90a1ab..cb06e362d6 100644 --- a/Userland/Applications/Spreadsheet/ImportDialog.h +++ b/Userland/Applications/Spreadsheet/ImportDialog.h @@ -66,6 +66,13 @@ private: RefPtr m_trim_leading_field_spaces_check_box; RefPtr m_trim_trailing_field_spaces_check_box; RefPtr m_data_preview_table_view; + RefPtr m_data_preview_error_label; + RefPtr m_data_preview_widget; + Vector m_quote_escape_items { + // Note: Keep in sync with Reader::ParserTraits::QuoteEscape. + "Repeat", + "Backslash", + }; }; struct ImportDialog { diff --git a/Userland/Applications/Spreadsheet/Readers/XSV.cpp b/Userland/Applications/Spreadsheet/Readers/XSV.cpp index adc28be508..8ed6ce9dac 100644 --- a/Userland/Applications/Spreadsheet/Readers/XSV.cpp +++ b/Userland/Applications/Spreadsheet/Readers/XSV.cpp @@ -105,6 +105,8 @@ Vector XSV::read_row(bool header_row) if (!header_row && (m_behaviours & ParserBehaviour::ReadHeaders) != ParserBehaviour::None && row.size() != m_names.size()) set_error(ReadError::NonConformingColumnCount); + else if (!header_row && !has_explicit_headers() && !m_rows.is_empty() && m_rows.first().size() != row.size()) + set_error(ReadError::NonConformingColumnCount); return row; } diff --git a/Userland/Applications/Spreadsheet/csv_import.gml b/Userland/Applications/Spreadsheet/csv_import.gml index d39b59aaad..be447f5fa3 100644 --- a/Userland/Applications/Spreadsheet/csv_import.gml +++ b/Userland/Applications/Spreadsheet/csv_import.gml @@ -168,8 +168,16 @@ margins: [10, 20, 10, 10] } - @GUI::TableView { - name: "data_preview_table_view" + @GUI::StackWidget { + name: "data_preview_widget" + + @GUI::TableView { + name: "data_preview_table_view" + } + @GUI::Label { + name: "data_preview_error_label" + word_wrap: true + } } } }