Replace place-holder strings

The NufxLib and diskimg libraries want narrow strings for certain
things, notably for the "storage name", i.e. how the name will appear
on the disk image or in the file archive.  We need to convert from
Windows UTF-16 to an Apple II filesystem-specific 8-bit character
representation.

We used to just pass narrow strings all the way through, so we didn't
need any intermediate storage to hold the conversion.  Now we do.  In
some cases there's nowhere good to put it.  The initial UTF-16
conversion changes just dropped in some place-holder strings.

This corrects the behavior, though in a couple of cases we're adding
kluges on top of code that was already badly bent from its original
intent (as initially conceived, CiderPress wasn't going to handle disk
images, just ShrinkIt archives).  It's not pretty, but it should work
for now.
This commit is contained in:
Andy McFadden 2014-12-04 17:05:48 -08:00
parent 66660a1fe4
commit d23a3b1ad8
8 changed files with 96 additions and 40 deletions

View File

@ -266,7 +266,7 @@ bool MainWindow::ChooseAddTarget(DiskImgLib::A2File** ppTargetSubdir,
*/
DiskArchive* pDiskArchive = (DiskArchive*) fpOpenArchive;
LOGI("Trying ChooseAddTarget");
LOGD("Trying ChooseAddTarget");
ChooseAddTargetDialog targetDialog(this);
targetDialog.fpDiskFS = pDiskArchive->GetDiskFS();
@ -1653,7 +1653,7 @@ void MainWindow::OnActionsRenameVolume(void)
result = fpOpenArchive->RenameVolume(this, rvDialog.fpChosenDiskFS,
rvDialog.fNewName);
if (!result) {
LOGI("RenameVolume FAILED");
LOGW("RenameVolume FAILED");
/* keep going -- reload just in case something partially happened */
}
@ -1933,7 +1933,7 @@ void MainWindow::OnUpdateActionsConvDisk(CCmdUI* pCmdUI)
/*
* ==========================================================================
* Convert to file archive
* Convert disk image to NuFX file archive
* ==========================================================================
*/
@ -2025,7 +2025,7 @@ void MainWindow::OnActionsConvFile(void)
fPreferences.SetPrefString(kPrOpenArchiveFolder, saveFolder);
filename = dlg.GetPathName();
LOGI(" Will xfer to file '%ls'", (LPCWSTR) filename);
LOGD(" Will xfer to file '%ls'", (LPCWSTR) filename);
/* remove file if it already exists */
CString errMsg;

View File

@ -40,6 +40,7 @@ private:
* archive.
*/
virtual BOOL OnInitDialog(void) override;
virtual void DoDataExchange(CDataExchange* pDX) override;
afx_msg void OnHelp(void);

View File

@ -1276,7 +1276,7 @@ NuError DiskArchive::DoAddFile(const AddFilesDialog* pAddOpts,
* on CreateFile's "make unique" feature and let the filesystem-specific
* code handle uniqueness.
*
* Any fields we want to keep from the NuFileDetails struct need to be
* Any fields we want to keep from the FileDetails struct need to be
* copied out. It's a "hairy" struct, so we need to duplicate the strings.
*/
NuError nuerr = kNuErrNone;
@ -1954,11 +1954,16 @@ bail:
return dierr;
}
// TODO: make this a member of FileDetails, and return a struct owned by
// FileDetails. This is necessary because we put const strings into
// pCreateParms that are owned by FileDetails, and need to coordinate the
// object lifetime.
void DiskArchive::ConvertFDToCP(const FileDetails* pDetails,
DiskFS::CreateParms* pCreateParms)
{
// TODO(xyzzy): need to store 8-bit form
pCreateParms->pathName = "XYZZY-DiskArchive"; // pDetails->storageName;
// ugly hack to get storage for narrow string
pDetails->fStorageNameA = pDetails->storageName;
pCreateParms->pathName = pDetails->fStorageNameA;
pCreateParms->fssep = (char) pDetails->fileSysInfo;
pCreateParms->storageType = pDetails->storageType;
pCreateParms->fileType = pDetails->fileType;

View File

@ -3,10 +3,8 @@
* Copyright (C) 2007 by faddenSoft, LLC. All Rights Reserved.
* See the file LICENSE for distribution terms.
*/
/*
* DiskFSTree implementation.
*/
#include "StdAfx.h"
#include "DiskFSTree.h"
#include "ChooseAddTargetDialog.h"
#include "HelpTopics.h"
@ -32,14 +30,22 @@ bool DiskFSTree::AddDiskFS(CTreeCtrl* pTree, HTREEITEM parent,
/*
* Insert an entry for the current item.
*
* The TVITEM struct wants a pointer to WCHAR* storage for the item
* text. The DiskFS only provides narrow chars, so we need to create
* some local storage for the widened version. Some calls that take a
* TVITEM allow the text to be edited, so the field is LPWSTR rather
* than LPCWSTR, but our uses don't allow editing, so we're okay
* passing it a pointer to storage inside a CString.
*/
pTarget = AllocTargetData();
pTarget->kind = kTargetDiskFS;
pTarget->pDiskFS = pDiskFS;
pTarget->pFile = NULL; // could also use volume dir for ProDOS
tvi.mask = TVIF_TEXT | TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_PARAM;
// TODO(xyzzy): need storage for wide-char version
tvi.pszText = L"XYZZY-DiskFSTree1"; // pDiskFS->GetVolumeID();
CString volumeIdW(pDiskFS->GetVolumeID()); // convert to wide string
int index = fStringHolder.Add(volumeIdW);
tvi.pszText = (LPWSTR)(LPCWSTR) fStringHolder.GetAt(index);
tvi.cchTextMax = 0; // not needed for insertitem
// tvi.iImage = kTreeImageFolderClosed;
// tvi.iSelectedImage = kTreeImageFolderOpen;
@ -57,7 +63,7 @@ bool DiskFSTree::AddDiskFS(CTreeCtrl* pTree, HTREEITEM parent,
tvins.hParent = parent;
hLocalRoot = pTree->InsertItem(&tvins);
if (hLocalRoot == NULL) {
LOGI("Tree root InsertItem failed");
LOGW("Tree root InsertItem failed");
return false;
}
@ -99,7 +105,6 @@ bool DiskFSTree::AddDiskFS(CTreeCtrl* pTree, HTREEITEM parent,
pTree->Select(hLocalRoot, TVGN_CARET);
}
return true;
}
@ -137,6 +142,8 @@ DiskImgLib::A2File* DiskFSTree::AddSubdir(CTreeCtrl* pTree, HTREEITEM parent,
} else {
/*
* Add an entry for this subdir (the "parent" entry).
*
* This has the same wide-vs-narrow storage issues as AddDiskFS().
*/
pTarget = AllocTargetData();
pTarget->kind = kTargetSubdir;
@ -144,8 +151,9 @@ DiskImgLib::A2File* DiskFSTree::AddSubdir(CTreeCtrl* pTree, HTREEITEM parent,
pTarget->pDiskFS = pDiskFS;
pTarget->pFile = pParentFile;
tvi.mask = TVIF_TEXT | TVIF_IMAGE | TVIF_SELECTEDIMAGE | TVIF_PARAM;
// TODO(xyzzy): need storage for wide-char version
tvi.pszText = L"XYZZY-DiskFSTree2"; // pParentFile->GetFileName();
CString fileNameW(pParentFile->GetFileName());
int index = fStringHolder.Add(fileNameW);
tvi.pszText = (LPWSTR)(LPCWSTR) fStringHolder.GetAt(index);
tvi.cchTextMax = 0; // not needed for insertitem
tvi.iImage = kTreeImageFolderClosed;
tvi.iSelectedImage = kTreeImageFolderOpen;
@ -155,7 +163,7 @@ DiskImgLib::A2File* DiskFSTree::AddSubdir(CTreeCtrl* pTree, HTREEITEM parent,
tvins.hParent = parent;
hLocalRoot = pTree->InsertItem(&tvins);
if (hLocalRoot == NULL) {
LOGI("Tree insert '%ls' failed", tvi.pszText);
LOGW("Tree insert '%ls' failed", tvi.pszText);
return NULL;
}
}
@ -191,7 +199,6 @@ DiskFSTree::TargetData* DiskFSTree::AllocTargetData(void)
if (pNew == NULL)
return NULL;
memset(pNew, 0, sizeof(*pNew));
/* insert it at the head of the list, and update the head pointer */
pNew->pNext = fpTargetData;

View File

@ -14,8 +14,13 @@
#include "../diskimg/DiskImg.h"
/*
* This class could probably be part of DiskArchive, but things are pretty
* cluttered up there already.
* Utility class for extracting a directory hierarchy from a DiskFS and
* adding it to a CTreeCtrl.
*
* The storage for some of the strings provided to the tree control is
* managed by this class, so delete this object after the CTreeCtrl is
* deleted. (Generally, this should be paired with a CTreeCtrl in a dialog
* object.)
*/
class DiskFSTree {
public:
@ -36,13 +41,18 @@ public:
/* if set, includes folders as well as disks */
bool fIncludeSubdirs;
/* start with the tree expanded to this depth (0=none, -1=all) */
int fExpandDepth;
typedef enum {
kTargetUnknown = 0, kTargetDiskFS, kTargetSubdir
} TargetKind;
typedef struct TargetData {
struct TargetData {
TargetData()
: kind(kTargetUnknown), selectable(false), pDiskFS(NULL),
pFile(NULL), pNext(NULL)
{}
TargetKind kind;
bool selectable;
DiskImgLib::DiskFS* pDiskFS;
@ -50,9 +60,9 @@ public:
// easier to keep a list than to chase through the tree
struct TargetData* pNext;
} TargetData;
};
private:
private:
/*
* Load the specified DiskFS into the tree, recursively adding any
* sub-volumes. Pass in an initial depth of 1.
@ -88,11 +98,16 @@ public:
*/
void FreeAllTargetData(void);
/*
* Load bitmaps used in the tree control.
*/
void LoadTreeImages(void) {
if (!fTreeImageList.Create(IDB_TREE_PICS, 16, 1, CLR_DEFAULT))
LOGI("GLITCH: list image create failed");
if (!fTreeImageList.Create(IDB_TREE_PICS, 16, 1, CLR_DEFAULT)) {
LOGW("GLITCH: list image create failed");
}
fTreeImageList.SetBkColor(::GetSysColor(COLOR_WINDOW));
}
enum { // defs for IDB_TREE_PICS
kTreeImageFolderClosed = 0,
kTreeImageFolderOpen = 1,
@ -103,6 +118,9 @@ public:
DiskImgLib::DiskFS* fpDiskFS;
TargetData* fpTargetData;
// Storage for wide strings that were converted from DiskFS narrow strings.
CStringArray fStringHolder;
};
#endif /*APP_DISKFSTREE_H*/

View File

@ -1073,7 +1073,7 @@ void PathProposal::LocalToArchive(const AddFilesDialog* pAddOpts)
if ((livePathStr[0] == '.' && livePathStr[1] == '.') ||
(wcsstr(livePathStr, slashDotDotSlash) != NULL))
{
LOGI("Found dot dot in '%ls', keeping only filename", livePathStr);
LOGD("Found dot dot in '%ls', keeping only filename", livePathStr);
doJunk = true;
}

View File

@ -970,17 +970,33 @@ GenericArchive::FileDetails::operator const NuFileDetails() const
break;
case kFileKindDirectory:
default:
LOGI("Invalid entryKind (%d) for NuFileDetails conversion",
LOGW("Invalid entryKind (%d) for NuFileDetails conversion",
entryKind);
ASSERT(false);
details.threadID = 0; // that makes it an old-style comment?!
break;
}
// TODO(xyzzy): need narrow-string versions of origName and storageName
// (probably need to re-think this automatic-cast-conversion stuff)
details.origName = "XYZZY-GenericArchive1"; // origName;
details.storageName = "XYZZY-GenericArchive2"; // storageName;
// The NuFileDetails origName field was added to NufxLib so that CiderPress
// could receive the original Windows pathname in the NuErrorStatus
// callback. This is a fairly compelling argument for making origName
// an opaque field.
//
// Whatever the case, we need to pass narrow-string versions of the
// names into NufxLib via the NuFileDetails struct. Since we're returning
// the NuFileDetails struct, the strings will out-live the scope of this
// function, so we have to store them somewhere else. There's nowhere
// in NuFileDetails, so we have to keep them in FileDetails, which currently
// exposes all of its fields publicly. I'm going to kluge it for now and
// just "snapshot" the wide strings into narrow-string fields. This is
// all pretty rickety and needs to be redone.
// TODO: make origName an opaque (void*) field in NufxLib and pass the
// wide char representation through
fOrigNameA = origName;
details.origName = fOrigNameA;
fStorageNameA = storageName;
details.storageName = fStorageNameA;
//details.fileSysID = fileSysID;
details.fileSysInfo = fileSysInfo;
details.access = access;
@ -1020,7 +1036,6 @@ GenericArchive::FileDetails::operator const NuFileDetails() const
break;
}
// Return stack copy, which copies into compiler temporary with our
// copy constructor.
return details;

View File

@ -385,9 +385,7 @@ public:
virtual bool GetReloadFlag(void) { return fReloadFlag; }
virtual void ClearReloadFlag(void) { fReloadFlag = false; }
// One of these for every sub-class. This is used to ensure that, should
// we need to down-cast an object, we did it correctly (without needing
// to include RTTI support).
// One of these for every sub-class.
typedef enum {
kArchiveUnknown = 0,
kArchiveNuFX,
@ -395,6 +393,8 @@ public:
kArchiveACU,
kArchiveDiskImage,
} ArchiveKind;
// Returns the kind of archive this is (disk image, NuFX, BNY, etc).
virtual ArchiveKind GetArchiveKind(void) = 0;
// Get a nice description for the title bar.
@ -520,9 +520,6 @@ public:
*
* It's based on the NuFileDetails class from NufxLib (which used to be
* used everywhere).
*
* TODO: xyzzy: NuFileDetails cast requires us to store pathnames with
* narrow strings.
*/
class FileDetails {
public:
@ -534,6 +531,12 @@ public:
* have a pointer to at least one of our strings, so structures
* filled out this way need to be short-lived. (Yes, this is
* annoying, but it's how NufxLib works.)
*
* TODO: make this a "GenNuFileDetails" method that returns a
* NuFileDetails struct owned by this object. It fills out the
* fields and references internal strings. Because we own the
* object, we can control the lifespan of the NuFileDetails, and
* ensure it doesn't live longer than our strings.
*/
operator const NuFileDetails() const;
@ -571,6 +574,7 @@ public:
/*
* Data fields. While transitioning from general use of NuFileDetails
* (v1.2.x to v2.0) I'm just going to leave these public.
* TODO: make this not public
*/
//NuThreadID threadID; /* data, rsrc, disk img? */
@ -584,8 +588,9 @@ public:
/*
* "Normalized" pathname. This is the full path with any of our
* added bits removed (e.g. file type & fork identifiers). It has
* not been sanitized for any specific target filesystem. See also
* PathProposal::LocalToArchive().
* not been sanitized for any specific target filesystem.
*
* This is generated by PathProposal::LocalToArchive().
*/
CString storageName;
@ -600,6 +605,11 @@ public:
NuDateTime modWhen;
NuDateTime archiveWhen;
// temporary kluge to get things working
mutable CStringA fOrigNameA;
mutable CStringA fStorageNameA;
private:
/*
* Copy the contents of our object to a new object.