Skip to content

Commit

Permalink
Improve handling of PDF/RTF/Word
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
fadden committed Jan 20, 2024
1 parent 6b46044 commit 0d778c3
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 19 deletions.
6 changes: 3 additions & 3 deletions AppCommon/ClipFileSet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 14 additions & 3 deletions AppCommon/ExtractFileWorker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
39 changes: 28 additions & 11 deletions cp2_wpf/FileViewer.xaml.cs
Original file line number Diff line number Diff line change
Expand Up @@ -731,20 +731,12 @@ private bool LaunchExternalViewer(Stream? stream, HostConv.FileKind kind) {
}

/// <summary>
/// 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.
/// </summary>
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 + "'");
Expand All @@ -755,6 +747,31 @@ private void DeleteTempFiles() {
}
}

/// <summary>
/// Generates a list of stale file viewer temporary files.
/// </summary>
public static List<string> FindStaleTempFiles() {
List<string> staleTemps = new List<string>();

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;
}

/// <summary>
/// Handles movement on the graphics zoom slider.
/// </summary>
Expand Down
29 changes: 29 additions & 0 deletions cp2_wpf/MainController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,8 @@ public void WindowLoaded() {
Debug.Assert(DiskArc.Disk.Woz_Meta.DebugTest());
AppHook.LogI("--- unit tests complete ---");

RemoveStaleTempFiles();

ApplyAppSettings();

UpdateTitle();
Expand Down Expand Up @@ -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<string> 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");
}

/// <summary>
/// Handles main window closing.
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions ndocs/Manual-cp2.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions ndocs/cli-tutorial/export.html
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ <h2>Exporting Files</h2>
random-access text files.</li>
<li>PNG bitmap (.PNG). Used for all graphics.</li>
</ol>
<p>Files viewable directly on a modern system, such as GIF images and PDF documents,
will be extracted without modification.</p>

<p>You can't specify conversion options when using "best" mode, but you can set
defaults in the config file (more on that later).</p>
Expand Down
7 changes: 5 additions & 2 deletions ndocs/gui-manual/viewing.html
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,11 @@ <h2>Conversions</h2>
<li>Bitmap. Used for anything graphical. Exports as .PNG.</li>
</ol>
<p>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.</p>
PNG files. These can be displayed directly by the Windows application, so there's no
need to convert them first. For <code>.pdf</code>, <code>.rtf</code>, 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 <code>.pdf</code>, <code>.rtf</code>, or
<code>.doc</code> filename extension.</p>

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

0 comments on commit 0d778c3

Please sign in to comment.