Hi all, In between bikeshedding with one hand, I have still been writing code with the other :) so anyway, here's the final version of the local package dir page browser and error-handling improvements fix; I'll hold off on committing it overnight while I try some more to break it and to give anyone who wants a chance to comment, at which point if nobody has raised anything I'll commit it without further ado.
setup/ChangeLog: * localdir.cc (offer_to_create): New function. (browse_cb): Handle selection changed and validate failed callbacks, and call offer_to_create where appropriate. (browse): Set new dialog style flags in browser info. (LocalDirPage::OnNext): Replace call to mkdir_p with offer to create or error message dialog display, allow proceeding to chooser even if local dir does not exist in unattend mode or if user insists, and fix small memory leak. * res.rc (IDS_MAYBE_MKDIR, IDS_CANT_MKDIR, IDS_NO_CWD): Define new string resources. * resource.h (IDS_MAYBE_MKDIR, IDS_CANT_MKDIR, IDS_NO_CWD): Define corresponding resource ID numbers. (I see that when there are no packages, the column headings don't size themselves correctly on entry to the chooser; I'll do something about that one too.) cheers, DaveK
Index: localdir.cc =================================================================== RCS file: /cvs/cygwin-apps/setup/localdir.cc,v retrieving revision 2.30 diff -p -u -r2.30 localdir.cc --- localdir.cc 4 Nov 2009 19:43:14 -0000 2.30 +++ localdir.cc 6 Dec 2009 21:56:44 -0000 @@ -116,16 +116,64 @@ save_dialog (HWND h) local_dir = egetString (h, IDC_LOCAL_DIR); } +// returns non-zero if refused or error, 0 if accepted and created ok. +static int +offer_to_create (HWND h, const char *dirname) +{ + if (!dirname || !*dirname) + return -1; + + if (!unattended_mode) + { + char msgText[MAX_PATH + 100]; + char fmtString[100]; + DWORD ret; + + LoadString (hinstance, IDS_MAYBE_MKDIR, fmtString, sizeof fmtString); + snprintf (msgText, sizeof msgText, fmtString, dirname); + + ret = MessageBox (h, msgText, 0, MB_ICONSTOP | MB_YESNO); + if (ret == IDNO) + return -1; + } + + int rv = mkdir_p (true, dirname, 0755); + + if (rv) + note (h, IDS_CANT_MKDIR, dirname); + + return rv; +} static int CALLBACK browse_cb (HWND h, UINT msg, LPARAM lp, LPARAM data) { + static CHAR dirname[MAX_PATH]; switch (msg) { case BFFM_INITIALIZED: if (local_dir.size()) SendMessage (h, BFFM_SETSELECTION, TRUE, (LPARAM) local_dir.c_str()); break; + case BFFM_SELCHANGED: + // Make a note of each new dir we successfully select, so that + // we know where to create the new directory if an invalid name + // is entered in the text box. + LPITEMIDLIST pidl = reinterpret_cast<LPITEMIDLIST>(lp); + SHGetPathFromIDList (pidl, dirname); + break; + case BFFM_VALIDATEFAILED: + // See if user wants to create a dir in the last successfully-selected. + CHAR tempname[MAX_PATH]; + snprintf (tempname, sizeof tempname, "%s\\%s", dirname, reinterpret_cast<LPTSTR>(lp)); + if (!offer_to_create (h, tempname)) + { + SendMessage (h, BFFM_SETSELECTION, TRUE, reinterpret_cast<LPARAM>(tempname)); + return -1; + } + // Reset to original directory instead. + SendMessage (h, BFFM_SETSELECTION, TRUE, reinterpret_cast<LPARAM>(dirname)); + return -1; } return 0; } @@ -141,7 +189,8 @@ browse (HWND h) bi.pszDisplayName = name; bi.lpszTitle = (source != IDC_SOURCE_CWD) ? "Select download directory" : "Select local package directory"; - bi.ulFlags = BIF_RETURNONLYFSDIRS; + bi.ulFlags = BIF_RETURNONLYFSDIRS | BIF_NEWDIALOGSTYLE + | ((source != IDC_SOURCE_CWD) ? (BIF_EDITBOX | BIF_VALIDATE) : 0); bi.lpfn = browse_cb; pidl = SHBrowseForFolder (&bi); if (pidl) @@ -199,10 +248,6 @@ LocalDirPage::OnNext () while (trySetCurDir) { trySetCurDir = false; - /* If we just install from local directory, the directory must already - exist. Otherwise, we just try to create it before we cd into it. */ - if (source != IDC_SOURCE_CWD) - mkdir_p (1, local_dir.c_str (), 0755); if (SetCurrentDirectoryA (local_dir.c_str())) { if (source == IDC_SOURCE_CWD) @@ -215,6 +260,25 @@ LocalDirPage::OnNext () return IDD_CHOOSE; } } + else if ((GetLastError () == ERROR_FILE_NOT_FOUND) + || (GetLastError () == ERROR_PATH_NOT_FOUND)) + { + if (source == IDC_SOURCE_CWD && unattended_mode) + return IDD_CHOOSE; + else if (source == IDC_SOURCE_CWD) + { + // Check the user really wants only to uninstall. + char msgText[1000]; + LoadString (hinstance, IDS_NO_CWD, msgText, sizeof (msgText)); + char msg[1000 + MAX_PATH]; + snprintf (msg, sizeof (msg), msgText, local_dir.c_str ()); + int ret = MessageBox (h, msg, 0, MB_ICONEXCLAMATION | MB_OKCANCEL); + return (ret == IDOK) ? IDD_CHOOSE : -1; + } + else if (offer_to_create (GetHWND (), local_dir.c_str ())) + return -1; + trySetCurDir = true; + } else { DWORD err = GetLastError (); @@ -228,6 +292,7 @@ LocalDirPage::OnNext () { snprintf (msg, sizeof (msg), msgText, local_dir.c_str(), buf, err); + LocalFree (buf); } else snprintf (msg, sizeof (msg), msgText, local_dir.c_str(), Index: res.rc =================================================================== RCS file: /cvs/cygwin-apps/setup/res.rc,v retrieving revision 2.81 diff -p -u -r2.81 res.rc --- res.rc 19 Sep 2009 03:38:50 -0000 2.81 +++ res.rc 6 Dec 2009 21:56:44 -0000 @@ -527,4 +527,8 @@ BEGIN "created if it does not already exist." IDS_LOCAL_DIR_INSTALL "Select a directory where Setup should look for " "downloaded installation files." + IDS_MAYBE_MKDIR "Directory %s does not exist, would you like me to create it?" + IDS_CANT_MKDIR "Couldn't create directory %s, sorry. (Is drive full or read-only?)" + IDS_NO_CWD "Local package directory %s not found.\nYou can still use setup.exe to remove installed\npackages, but there " + "will be nothing to install.\n\nPress OK if that's what you wanted\nor Cancel to choose a different directory." END Index: resource.h =================================================================== RCS file: /cvs/cygwin-apps/setup/resource.h,v retrieving revision 2.39 diff -p -u -r2.39 resource.h --- resource.h 11 May 2009 20:32:59 -0000 2.39 +++ resource.h 6 Dec 2009 21:56:44 -0000 @@ -36,6 +36,9 @@ #define IDS_SEARCH_TOOLTIP 133 #define IDS_LOCAL_DIR_DOWNLOAD 134 #define IDS_LOCAL_DIR_INSTALL 135 +#define IDS_MAYBE_MKDIR 136 +#define IDS_CANT_MKDIR 137 +#define IDS_NO_CWD 138 // Dialogs