mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 13:42:44 +00:00 
			
		
		
		
	Spreadsheet: Generate file previews in memory and save directly to file
When we were asked to make a new preview text, we first generated the whole file in the /tmp directory and then read the first 8 lines to put them in a text box. This was doing a bit of unnecessary work at first, but more importantly, didn't work on a given file descriptor from FileSystemAccessServer as we were copying a file to a filename instead, meaning that we also had to unveil the whole home directory. Anyway, previews will be written now to a MemoryStream by the generator, which will limit to 8 lines. File saves will omit /tmp entirely, allowing us to tighten program unveil list a little. :^)
This commit is contained in:
		
							parent
							
								
									c74441395b
								
							
						
					
					
						commit
						29a3cdcfb7
					
				
					 4 changed files with 60 additions and 113 deletions
				
			
		|  | @ -10,6 +10,7 @@ | |||
| #include <AK/DeprecatedString.h> | ||||
| #include <AK/JsonArray.h> | ||||
| #include <AK/LexicalPath.h> | ||||
| #include <AK/MemoryStream.h> | ||||
| #include <Applications/Spreadsheet/CSVExportGML.h> | ||||
| #include <LibCore/File.h> | ||||
| #include <LibCore/FileStream.h> | ||||
|  | @ -35,21 +36,6 @@ CSVExportDialogPage::CSVExportDialogPage(Sheet const& sheet) | |||
| { | ||||
|     m_headers.extend(m_data.take_first()); | ||||
| 
 | ||||
|     auto temp_template = DeprecatedString::formatted("{}/spreadsheet-csv-export.{}.XXXXXX", Core::StandardPaths::tempfile_directory(), getpid()); | ||||
|     auto temp_path = ByteBuffer::create_uninitialized(temp_template.length() + 1).release_value_but_fixme_should_propagate_errors(); | ||||
|     auto buf = reinterpret_cast<char*>(temp_path.data()); | ||||
|     auto copy_ok = temp_template.copy_characters_to_buffer(buf, temp_path.size()); | ||||
|     VERIFY(copy_ok); | ||||
| 
 | ||||
|     int fd = mkstemp(buf); | ||||
|     if (fd < 0) { | ||||
|         perror("mkstemp"); | ||||
|         // Just let the operation fail cleanly later.
 | ||||
|     } else { | ||||
|         unlink(buf); | ||||
|         m_temp_output_file_path = temp_path; | ||||
|     } | ||||
| 
 | ||||
|     m_page = GUI::WizardPage::construct( | ||||
|         "CSV Export Options", | ||||
|         "Please select the options for the csv file you wish to export to"); | ||||
|  | @ -104,7 +90,7 @@ CSVExportDialogPage::CSVExportDialogPage(Sheet const& sheet) | |||
|     update_preview(); | ||||
| } | ||||
| 
 | ||||
| auto CSVExportDialogPage::make_writer() -> Optional<XSV> | ||||
| auto CSVExportDialogPage::make_writer(OutputStream& stream) -> Optional<XSV> | ||||
| { | ||||
|     DeprecatedString delimiter; | ||||
|     DeprecatedString quote; | ||||
|  | @ -167,80 +153,24 @@ auto CSVExportDialogPage::make_writer() -> Optional<XSV> | |||
|     if (should_quote_all_fields) | ||||
|         behaviors = behaviors | Writer::WriterBehavior::QuoteAll; | ||||
| 
 | ||||
|     // Note that the stream is used only by the ctor.
 | ||||
|     auto stream = Core::OutputFileStream::open(m_temp_output_file_path); | ||||
|     if (stream.is_error()) { | ||||
|         dbgln("Cannot open {} for writing: {}", m_temp_output_file_path, stream.error()); | ||||
|         return {}; | ||||
|     } | ||||
|     XSV writer(stream.value(), m_data, traits, *headers, behaviors); | ||||
| 
 | ||||
|     if (stream.value().has_any_error()) { | ||||
|         dbgln("Write error when making preview"); | ||||
|         return {}; | ||||
|     } | ||||
| 
 | ||||
|     return writer; | ||||
|     return XSV(stream, m_data, move(traits), *headers, behaviors); | ||||
| } | ||||
| 
 | ||||
| void CSVExportDialogPage::update_preview() | ||||
| 
 | ||||
| { | ||||
|     m_previously_made_writer = make_writer(); | ||||
|     if (!m_previously_made_writer.has_value()) { | ||||
|     fail:; | ||||
|     DuplexMemoryStream memory_stream; | ||||
|     auto maybe_writer = make_writer(memory_stream); | ||||
|     if (!maybe_writer.has_value()) { | ||||
|         m_data_preview_text_editor->set_text({}); | ||||
|         return; | ||||
|     } | ||||
| 
 | ||||
|     auto file_or_error = Core::File::open( | ||||
|         m_temp_output_file_path, | ||||
|         Core::OpenMode::ReadOnly); | ||||
|     if (file_or_error.is_error()) | ||||
|         goto fail; | ||||
| 
 | ||||
|     auto& file = *file_or_error.value(); | ||||
|     StringBuilder builder; | ||||
|     size_t line = 0; | ||||
|     while (file.can_read_line()) { | ||||
|         if (++line == 8) | ||||
|             break; | ||||
| 
 | ||||
|         builder.append(file.read_line()); | ||||
|         builder.append('\n'); | ||||
|     } | ||||
|     m_data_preview_text_editor->set_text(builder.string_view()); | ||||
|     maybe_writer->generate_preview(); | ||||
|     auto buffer = memory_stream.copy_into_contiguous_buffer(); | ||||
|     m_data_preview_text_editor->set_text(StringView(buffer)); | ||||
|     m_data_preview_text_editor->update(); | ||||
| } | ||||
| 
 | ||||
| Result<void, DeprecatedString> CSVExportDialogPage::move_into(DeprecatedString const& target) | ||||
| { | ||||
|     auto& source = m_temp_output_file_path; | ||||
| 
 | ||||
|     // First, try rename().
 | ||||
|     auto rc = rename(source.characters(), target.characters()); | ||||
|     if (rc == 0) | ||||
|         return {}; | ||||
| 
 | ||||
|     auto saved_errno = errno; | ||||
|     if (saved_errno == EXDEV) { | ||||
|         // Can't do that, copy it instead.
 | ||||
|         auto result = Core::File::copy_file_or_directory( | ||||
|             target, source, | ||||
|             Core::File::RecursionMode::Disallowed, | ||||
|             Core::File::LinkMode::Disallowed, | ||||
|             Core::File::AddDuplicateFileMarker::No); | ||||
| 
 | ||||
|         if (result.is_error()) | ||||
|             return DeprecatedString::formatted("{}", static_cast<Error const&>(result.error())); | ||||
| 
 | ||||
|         return {}; | ||||
|     } | ||||
| 
 | ||||
|     perror("rename"); | ||||
|     return DeprecatedString { strerror(saved_errno) }; | ||||
| } | ||||
| 
 | ||||
| Result<void, DeprecatedString> ExportDialog::make_and_run_for(StringView mime, Core::File& file, Workbook& workbook) | ||||
| { | ||||
|     auto wizard = GUI::WizardDialog::construct(GUI::Application::the()->active_window()); | ||||
|  | @ -255,20 +185,17 @@ Result<void, DeprecatedString> ExportDialog::make_and_run_for(StringView mime, C | |||
| 
 | ||||
|         CSVExportDialogPage page { workbook.sheets().first() }; | ||||
|         wizard->replace_page(page.page()); | ||||
|         auto result = wizard->exec(); | ||||
| 
 | ||||
|         if (result == GUI::Dialog::ExecResult::OK) { | ||||
|             auto& writer = page.writer(); | ||||
|             if (!writer.has_value()) | ||||
|                 return DeprecatedString { "CSV Export failed" }; | ||||
|             if (writer->has_error()) | ||||
|                 return DeprecatedString::formatted("CSV Export failed: {}", writer->error_string()); | ||||
| 
 | ||||
|             // No error, move the temp file to the expected location
 | ||||
|             return page.move_into(file.filename()); | ||||
|         } else { | ||||
|         if (wizard->exec() != GUI::Dialog::ExecResult::OK) | ||||
|             return DeprecatedString { "CSV Export was cancelled" }; | ||||
|         } | ||||
| 
 | ||||
|         auto file_stream = Core::OutputFileStream(file); | ||||
|         auto writer = page.make_writer(file_stream); | ||||
|         if (!writer.has_value()) | ||||
|             return DeprecatedString::formatted("CSV Export failed"); | ||||
|         writer->generate(); | ||||
|         if (writer->has_error()) | ||||
|             return DeprecatedString::formatted("CSV Export failed: {}", writer->error_string()); | ||||
|         return {}; | ||||
|     }; | ||||
| 
 | ||||
|     auto export_worksheet = [&]() -> Result<void, DeprecatedString> { | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Karol Kosek
						Karol Kosek