From 0d778c32b3bc4af4f1148c6e5aeec3595f613eab Mon Sep 17 00:00:00 2001 From: Andy McFadden Date: Sat, 20 Jan 2024 09:46:04 -0800 Subject: [PATCH] Improve handling of PDF/RTF/Word The file viewer can't clean up its temporary files if an external viewer still has the file open, so we need to scan for stale temp files when the program starts. We're removing "cp2tmp_*" from the system temporary directory, so it should be safe to remove the files without user review and confirmation. The CLI was refusing to export "host image" files, but this seems wrong. We now simply extract them as-is. The GUI was already doing this. Fixed a bug where you couldn't export a "host image" file from the GUI with drag & drop if it was in a file archive. (Issue #7) --- AppCommon/ClipFileSet.cs | 6 +++--- AppCommon/ExtractFileWorker.cs | 17 ++++++++++++--- cp2_wpf/FileViewer.xaml.cs | 39 ++++++++++++++++++++++++---------- cp2_wpf/MainController.cs | 29 +++++++++++++++++++++++++ ndocs/Manual-cp2.md | 2 ++ ndocs/cli-tutorial/export.html | 2 ++ ndocs/gui-manual/viewing.html | 7 ++++-- 7 files changed, 83 insertions(+), 19 deletions(-) diff --git a/AppCommon/ClipFileSet.cs b/AppCommon/ClipFileSet.cs index 46aff74..876d503 100644 --- a/AppCommon/ClipFileSet.cs +++ b/AppCommon/ClipFileSet.cs @@ -733,9 +733,9 @@ private void CreateForExport(object archiveOrFileSystem, IFileEntry entry, PNGGenerator.Generate((IBitmap)convOutput, outStream); } else if (convOutput is HostConv) { // Copy directly to output. - if (dataStream != null) { - dataStream.Position = 0; - dataStream.CopyTo(outStream); + if (dataCopy != null) { + dataCopy.Position = 0; + dataCopy.CopyTo(outStream); } } else { Debug.Assert(false, "unknown IConvOutput impl " + convOutput); diff --git a/AppCommon/ExtractFileWorker.cs b/AppCommon/ExtractFileWorker.cs index 5970d24..10f3b72 100644 --- a/AppCommon/ExtractFileWorker.cs +++ b/AppCommon/ExtractFileWorker.cs @@ -700,9 +700,20 @@ private bool DoExport(string extractPath, FileAttribs attrs, Stream? dataStream, PNGGenerator.Generate((IBitmap)convOutput, outStream); } } else if (convOutput is HostConv) { - // Should we just copy these to the output unmodified? - ReportConvFailure("GIF/JPEG/PNG should be extracted, not exported"); - return false; + // Copy these to the output unmodified + if (dataCopy == null) { + ReportConvFailure("weird: HostConv but no data fork"); + return false; + } + if (!PrepareOutputFile(extractPath, out bool doCancel)) { + return !doCancel; + } + cleanPath1 = extractPath; + ShowProgress(attrs, cleanPath1, false, dataPercent, conv.Tag); + using (Stream outStream = new FileStream(cleanPath1, FileMode.CreateNew)) { + dataCopy.Position = 0; + dataCopy.CopyTo(outStream); + } } else { Debug.Assert(false, "unknown IConvOutput impl " + convOutput); ReportConvFailure("got weird output object: " + convOutput); diff --git a/cp2_wpf/FileViewer.xaml.cs b/cp2_wpf/FileViewer.xaml.cs index 0a259ad..78f8b33 100644 --- a/cp2_wpf/FileViewer.xaml.cs +++ b/cp2_wpf/FileViewer.xaml.cs @@ -731,20 +731,12 @@ private bool LaunchExternalViewer(Stream? stream, HostConv.FileKind kind) { } /// - /// Removes temporary files we created. Call this when the file viewer is closed. + /// Removes temporary files we created. Call this when the file viewer is closed. Any + /// files still open in external programs will not be deleted. /// private void DeleteTempFiles() { - // TODO: we probably also want to scrub any leftovers from previous iterations - // when the program launches. Scan of local temp directory should be fast, though - // we might want to put it in a dedicated directory. Should get user confirmation, - // ideally with a non-intrusive launch screen affordance rather than a modal - // dialog. If we can open the file read/write then the external viewer should be - // done with it. - string tempPathRoot = Path.GetTempPath(); foreach (string path in mTmpFiles) { - if (!path.StartsWith(tempPathRoot)) { - throw new Exception("whoops"); // be paranoid about removing files - } + Debug.Assert(path.StartsWith(Path.GetTempPath())); try { File.Delete(path); mAppHook.LogI("Removed temp '" + path + "'"); @@ -755,6 +747,31 @@ private void DeleteTempFiles() { } } + /// + /// Generates a list of stale file viewer temporary files. + /// + public static List FindStaleTempFiles() { + List staleTemps = new List(); + + string pattern = TEMP_FILE_PREFIX + "*"; + string[] allFiles = Directory.GetFiles(Path.GetTempPath(), pattern); + + // Check to see if the file is still open, by opening it read-only with sharing + // disallowed. There's a potential race condition, where something else re-opens + // it after our test, but that's not really interesting for us. + foreach (string path in allFiles) { + try { + using (FileStream stream = new FileStream(path, FileMode.Open, + FileAccess.Read, FileShare.None)) { + staleTemps.Add(path); + } + } catch { + // If we can't open it, we probably can't delete it either. + } + } + return staleTemps; + } + /// /// Handles movement on the graphics zoom slider. /// diff --git a/cp2_wpf/MainController.cs b/cp2_wpf/MainController.cs index bc1d7f5..a1d4770 100644 --- a/cp2_wpf/MainController.cs +++ b/cp2_wpf/MainController.cs @@ -155,6 +155,8 @@ public void WindowLoaded() { Debug.Assert(DiskArc.Disk.Woz_Meta.DebugTest()); AppHook.LogI("--- unit tests complete ---"); + RemoveStaleTempFiles(); + ApplyAppSettings(); UpdateTitle(); @@ -186,6 +188,33 @@ private void ProcessCommandLine() { } } + private void RemoveStaleTempFiles() { + // Ideally we'd prompt the user before doing this, but the odds of something useful + // living in the system temp directory with our app-specific filename prefix are + // very low. If we want to be cautious, we can add a "stale temp files found" item + // to the launch screen, with "view" and "clear" buttons. + + DateTime startWhen = DateTime.Now; + List staleTemps = FileViewer.FindStaleTempFiles(); + foreach (string filePath in staleTemps) { + if (!filePath.StartsWith(Path.GetTempPath())) { + // Be paranoid when removing files. + Debug.Assert(false); + throw new Exception("Internal error: bad temp path " + filePath); + } + try { + File.Delete(filePath); + AppHook.LogI("Removed stale temp file '" + filePath + "'"); + } catch (Exception ex) { + AppHook.LogW("Failed to delete temp file: " + ex.Message); + } + } + + DateTime endWhen = DateTime.Now; + AppHook.LogD("Stale temp file scan completed in " + + (endWhen - startWhen).TotalSeconds + " ms"); + } + /// /// Handles main window closing. /// diff --git a/ndocs/Manual-cp2.md b/ndocs/Manual-cp2.md index 4c3c079..704a81f 100644 --- a/ndocs/Manual-cp2.md +++ b/ndocs/Manual-cp2.md @@ -1635,6 +1635,8 @@ The result of the conversion takes one of the following forms: - Formatted documents are output in Rich Text Format (.RTF). - Spreadsheets and other cell-grid formats are output as Comma-Separated Value (.CSV) files, UTF-8 encoded. + - Files that can be used on a modern system, such as GIF images and PDF + documents, are extracted without modification. Specifying the special value `best` as the converter tag will analyze the file and choose the conversion that seems most appropriate. It's not possible to diff --git a/ndocs/cli-tutorial/export.html b/ndocs/cli-tutorial/export.html index 3e6052b..22a41e3 100644 --- a/ndocs/cli-tutorial/export.html +++ b/ndocs/cli-tutorial/export.html @@ -126,6 +126,8 @@

Exporting Files

random-access text files.
  • PNG bitmap (.PNG). Used for all graphics.
    • +

    Files viewable directly on a modern system, such as GIF images and PDF documents, +will be extracted without modification.

    You can't specify conversion options when using "best" mode, but you can set defaults in the config file (more on that later).

    diff --git a/ndocs/gui-manual/viewing.html b/ndocs/gui-manual/viewing.html index db99ec5..2c036c6 100644 --- a/ndocs/gui-manual/viewing.html +++ b/ndocs/gui-manual/viewing.html @@ -121,8 +121,11 @@

    Conversions

  • Bitmap. Used for anything graphical. Exports as .PNG.

    • Technically there is a fifth type, "host image", which is used for GIF, JPEG, and -PNG files. These can be displayed directly by the host system, so there's no need to -convert them first.

    +PNG files. These can be displayed directly by the Windows application, so there's no +need to convert them first. For .pdf, .rtf, and Word documents +identified by HFS file type, the file will be extracted to a temporary file and displayed +with the system's default handler for the .pdf, .rtf, or +.doc filename extension.

    The set of conversions that can be applied to a file are determined automatically, and placed in the Conversion pop-up menu. They will be sorted by