Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Save dialog #143

Closed
wants to merge 12 commits into from

3 participants

@ZweiSteinSoft

No description provided.

@ZweiSteinSoft ZweiSteinSoft referenced this pull request in adobe/brackets
Closed

Save as #2116

appshell/appshell_extensions_win.cpp
((35 lines not shown))
+ OPENFILENAME ofn;
+
+ ZeroMemory(&ofn, sizeof(ofn));
+ ofn.hwndOwner = GetActiveWindow();
+ ofn.lStructSize = sizeof(ofn);
+ ofn.lpstrFile = szFile;
+ ofn.nMaxFile = MAX_PATH;
+
+ // TODO (issue #65) - Use passed in file types. Note, when fileTypesStr is null, all files should be shown
+ /* findAndReplaceString( fileTypesStr, std::string(" "), std::string(";*."));
+ LPCWSTR allFilesFilter = L"All Files\0*.*\0\0";*/
+
+ ofn.lpstrFilter = L"All Files\0*.*\0Web Files\0*.js;*.css;*.htm;*.html\0\0";
+
+ ofn.lpstrInitialDir = initialDirectory.c_str();
+ ofn.Flags = OFN_PATHMUSTEXIST | OFN_FILEMUSTEXIST | OFN_NOCHANGEDIR | OFN_EXPLORER;
@peterflynn Owner

These flags don't look quite right. The docs say OFN_FILEMUSTEXIST cannot be used with a Save As dialog. Not sure if OFN_PATHMUSTEXIST is right either -- does it simply prevent entering files whose containing folder(s) don't exist, or does it block any nonexistent filename too?

It also seems like OFN_OVERWRITEPROMPT should be set (unless it's on by default for Save As, but the docs don't say that).

For OFN_PATHMUSTEXIST, the docs don't say that it would only be valid in an Open File Dialog.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gruehle gruehle was assigned
appshell/appshell_extensions.cpp
((5 lines not shown))
+ // Parameters:
+ // 0: int32 - callback id
+ // 1: string - title
+ // 2: string - initialPath
+ // 3: string - fileTypes (space-delimited string)
+ if (argList->GetSize() != 4 ||
+ argList->GetType(1) != VTYPE_STRING ||
+ argList->GetType(2) != VTYPE_STRING ||
+ argList->GetType(3) != VTYPE_STRING) {
+ error = ERR_INVALID_PARAMS;
+ }
+
+ CefRefPtr<CefListValue> selectedFile = CefListValue::Create();
+
+ if (error == NO_ERROR) {
+ ExtensionString title = argList->GetString(3);
@gruehle Owner
gruehle added a note

These three lines are getting strings 3-5. I think you want to get strings 1-3 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gruehle gruehle commented on the diff
appshell/appshell_extensions.cpp
((12 lines not shown))
+ argList->GetType(2) != VTYPE_STRING ||
+ argList->GetType(3) != VTYPE_STRING) {
+ error = ERR_INVALID_PARAMS;
+ }
+
+ CefRefPtr<CefListValue> selectedFile = CefListValue::Create();
+
+ if (error == NO_ERROR) {
+ ExtensionString title = argList->GetString(3);
+ ExtensionString initialPath = argList->GetString(4);
+ ExtensionString fileTypes = argList->GetString(5);
+
+ error = ShowSaveDialog(title,
+ initialPath,
+ fileTypes,
+ selectedFile);
@gruehle Owner
gruehle added a note

Since ShowSaveDialog will only return a single item, it would be better to pass in a reference to an ExtensionString rather than a list.

How would I do this? With or without the CefRefPtr? And how would I initialize the ExtensionString?

@gruehle Owner
gruehle added a note

ExtensionString is just a std::string on the Mac and std::wstring on Windows. It would be easiest to set the initial value in the ShowSaveDialog function, since you need to set it to "" on the mac and 'L""` on windows (for wide string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions.js
@@ -130,6 +130,33 @@ if (!appshell.app) {
};
/**
+ * Display the OS File Save dialog, allowing the user to select
+ * a file or directory.
@gruehle Owner
gruehle added a note

Comment should be updated to say "save" instead of "select" and remove "or directory".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions.js
@@ -130,6 +130,33 @@ if (!appshell.app) {
};
/**
+ * Display the OS File Save dialog, allowing the user to select
+ * a file or directory.
+ *
+ * @param {string} title Tile of the save dialog.
+ * @param {string} initialPath Initial path to display in the dialog. Pass NULL or "" to
+ * display the last path chosen.
+ * @param {Array.<string>} fileTypes Array of strings specifying the selectable file extensions.
+ * These strings should not contain '.'. This parameter is ignored when
+ * chooseDirectory=true.
@gruehle Owner
gruehle added a note

Last sentence ("This parameter is ignored when...") should be removed since this function doesn't have a chooseDirectory parameter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions.js
@@ -130,6 +130,33 @@ if (!appshell.app) {
};
/**
+ * Display the OS File Save dialog, allowing the user to select
+ * a file or directory.
+ *
+ * @param {string} title Tile of the save dialog.
+ * @param {string} initialPath Initial path to display in the dialog. Pass NULL or "" to
+ * display the last path chosen.
+ * @param {Array.<string>} fileTypes Array of strings specifying the selectable file extensions.
+ * These strings should not contain '.'. This parameter is ignored when
+ * chooseDirectory=true.
+ * @param {function(err, selection)} callback Asynchronous callback function. The callback gets two arguments
+ * (err, selection) where selection is an array of the names of the selected files.
@gruehle Owner
gruehle added a note

Does this return an Array or a String?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions_mac.mm
((6 lines not shown))
+ ExtensionString fileTypes,
+ CefRefPtr<CefListValue>& selectedFile)
+{
+ NSArray* allowedFileTypes = nil;
+
+ if (fileTypes != "")
+ {
+ // fileTypes is a Space-delimited string
+ allowedFileTypes =
+ [[NSString stringWithUTF8String:fileTypes.c_str()]
+ componentsSeparatedByString:@" "];
+ }
+
+ // Initialize the dialog
+ NSSavePanel* savePanel = [NSSavePanel savePanel];
+ [savePanel setCanCreateDirectories: YES];
@gruehle Owner
gruehle added a note

Where is canChooseDirectories declared? I don't see that variable in this function.

The line doesn't mention or use canChooseDirectories —possibly looked at the wrong column of your diff?

@gruehle Owner
gruehle added a note

Weird. When I first looked at the diffs, this line said '[savePanel setCanCreateDirectories: canChooseDirectories];`. Yes, this is fine. Please ignore my comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gruehle gruehle commented on the diff
appshell/appshell_extensions_mac.mm
((23 lines not shown))
+ [savePanel setTitle: [NSString stringWithUTF8String:title.c_str()]];
+
+ if (initialDirectory != "")
+ [savePanel setDirectoryURL:[NSURL URLWithString:[NSString stringWithUTF8String:initialDirectory.c_str()]]];
+
+ [savePanel setAllowedFileTypes:allowedFileTypes];
+
+ [savePanel beginSheetModalForWindow:[NSApp mainWindow] completionHandler:nil];
+ if ([savePanel runModal] == NSOKButton)
+ {
+ selectedFile->SetString(0, [[[savePanel URL] path] UTF8String]);
+ }
+ [NSApp endSheet:savePanel];
+
+ return NO_ERROR;
+}
@gruehle Owner
gruehle added a note

If the user cancels the dialog, is the return value being set to ""?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions_win.cpp
@@ -510,6 +510,60 @@ int32 ShowOpenDialog(bool allowMultipleSelection,
return NO_ERROR;
}
+int32 ShowSaveDialog(ExtensionString title,
+ ExtensionString initialDirectory,
+ ExtensionString fileTypes,
+ CefRefPtr<CefListValue>& selectedFile)
+{
+ wchar_t szFile[MAX_PATH];
+ szFile[0] = 0;
+
+ // TODO (issue #64) - This method should be using IFileDialog instead of the
+ /* outdated SHGetPathFromIDList and GetOpenFileName.
@gruehle Owner
gruehle added a note

This comment should be updated or removed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
appshell/appshell_extensions_win.cpp
((38 lines not shown))
+ ofn.lStructSize = sizeof(ofn);
+ ofn.lpstrFile = szFile;
+ ofn.nMaxFile = MAX_PATH;
+
+ // TODO (issue #65) - Use passed in file types. Note, when fileTypesStr is null, all files should be shown
+ /* findAndReplaceString( fileTypesStr, std::string(" "), std::string(";*."));
+ LPCWSTR allFilesFilter = L"All Files\0*.*\0\0";*/
+
+ ofn.lpstrFilter = L"All Files\0*.*\0Web Files\0*.js;*.css;*.htm;*.html\0\0";
+
+ ofn.lpstrInitialDir = initialDirectory.c_str();
+ ofn.Flags = OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT | OFN_NOCHANGEDIR | OFN_EXPLORER;
+
+ if (GetSaveFileName(&ofn)) {
+ selectedFile->SetString(0, szFile);
+ }
@gruehle Owner
gruehle added a note

Where is the return value set if the user cancels the dialog?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@gruehle
Owner

Done with initial review. Thanks for submitting!

@gruehle
Owner

You don't need to wrap the ExtensionString in a CefRefPtr -- using ExtensionString& selectedFile is all you need.

@ZweiSteinSoft

@gruehle Is everything okay now? (See adobe/brackets#2116, too.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 14, 2012
  1. Add save dialog

    J.M authored
  2. Add save-as for mac

    J.M authored
  3. Use CefRefPtr<CefListValue> for save dialog, for now; drop folder-sel…

    J.M authored
    …ection support for save dialog
  4. Add @echo off to make_symlinks.bat

    J.M authored
  5. Use SetList for ShowSaveDialog, too

    J.M authored
  6. Return single string only

    J.M authored
Commits on Nov 27, 2012
Commits on Nov 29, 2012
  1. Fix NSSaveDialog

    J.M authored
This page is out of date. Refresh to see the latest.
View
28 appshell/appshell_extensions.cpp
@@ -131,6 +131,34 @@ class ProcessMessageDelegate : public ClientHandler::ProcessMessageDelegate {
// Set response args for this function
responseArgs->SetList(2, selectedFiles);
+ } else if (message_name == "ShowSaveDialog") {
+ // Parameters:
+ // 0: int32 - callback id
+ // 1: string - title
+ // 2: string - initialPath
+ // 3: string - fileTypes (space-delimited string)
+ if (argList->GetSize() != 4 ||
+ argList->GetType(1) != VTYPE_STRING ||
+ argList->GetType(2) != VTYPE_STRING ||
+ argList->GetType(3) != VTYPE_STRING) {
+ error = ERR_INVALID_PARAMS;
+ }
+
+ ExtensionString selectedFile;
+
+ if (error == NO_ERROR) {
+ ExtensionString title = argList->GetString(1);
+ ExtensionString initialPath = argList->GetString(2);
+ ExtensionString fileTypes = argList->GetString(3);
+
+ error = ShowSaveDialog(title,
+ initialPath,
+ fileTypes,
+ selectedFile);
@gruehle Owner
gruehle added a note

Since ShowSaveDialog will only return a single item, it would be better to pass in a reference to an ExtensionString rather than a list.

How would I do this? With or without the CefRefPtr? And how would I initialize the ExtensionString?

@gruehle Owner
gruehle added a note

ExtensionString is just a std::string on the Mac and std::wstring on Windows. It would be easiest to set the initial value in the ShowSaveDialog function, since you need to set it to "" on the mac and 'L""` on windows (for wide string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+
+ // Set response args for this function
+ responseArgs->SetString(2, selectedFile);
} else if (message_name == "ReadDir") {
// Parameters:
// 0: int32 - callback id
View
25 appshell/appshell_extensions.js
@@ -130,6 +130,31 @@ if (!appshell.app) {
};
/**
+ * Display the OS File Save dialog, allowing the user to save a file.
+ *
+ * @param {string} title Tile of the save dialog.
+ * @param {string} initialPath Initial path to display in the dialog. Pass NULL or "" to
+ * display the last path chosen.
+ * @param {Array.<string>} fileTypes Array of strings specifying the selectable file extensions.
+ * These strings should not contain '.'.
+ * @param {function(err, selection)} callback Asynchronous callback function. The callback gets two arguments
+ * (err, selection) where selection is the name of the file to save.
+ * Possible error values:
+ * NO_ERROR
+ * ERR_INVALID_PARAMS
+ *
+ * @return None. This is an asynchronous call that sends all return information to the callback.
+ */
+ native function ShowSaveDialog();
+ appshell.fs.showSaveDialog = function (title, initialPath, fileTypes, callback) {
+ setTimeout(function () {
+ ShowSaveDialog(callback,
+ title || 'Save As', initialPath || '',
+ fileTypes ? fileTypes.join(' ') : '');
+ }, 10);
+ };
+
+ /**
* Reads the contents of a directory.
*
* @param {string} path The path of the directory to read.
View
39 appshell/appshell_extensions_mac.mm
@@ -326,6 +326,45 @@ int32 ShowOpenDialog(bool allowMulitpleSelection,
return NO_ERROR;
}
+int32 ShowSaveDialog(ExtensionString title,
+ ExtensionString initialDirectory,
+ ExtensionString fileTypes,
+ ExtensionString& selectedFile)
+{
+ NSArray* allowedFileTypes = nil;
+
+ if (fileTypes != "")
+ {
+ // fileTypes is a Space-delimited string
+ allowedFileTypes =
+ [[NSString stringWithUTF8String:fileTypes.c_str()]
+ componentsSeparatedByString:@" "];
+ }
+
+ selectedFile = "";
+
+ // Initialize the dialog
+ NSSavePanel* savePanel = [NSSavePanel savePanel];
+ [savePanel setCanCreateDirectories: YES];
+ [savePanel setShowsHiddenFiles: YES];
+ [savePanel setTitle: [NSString stringWithUTF8String:title.c_str()]];
+
+ if (initialDirectory != "")
+ [savePanel setDirectoryURL:[NSURL URLWithString:[NSString stringWithUTF8String:initialDirectory.c_str()]]];
+
+ [savePanel setAllowedFileTypes:allowedFileTypes];
+
+ [savePanel beginSheetModalForWindow:[NSApp mainWindow] completionHandler:nil];
+
+ if ([savePanel runModal] == NSOKButton)
+ {
+ selectedFile = [[[savePanel URL] path] UTF8String];
+ }
+ [NSApp endSheet:savePanel];
+
+ return NO_ERROR;
+}
@gruehle Owner
gruehle added a note

If the user cancels the dialog, is the return value being set to ""?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+
int32 ReadDir(ExtensionString path, CefRefPtr<CefListValue>& directoryContents)
{
NSString* pathStr = [NSString stringWithUTF8String:path.c_str()];
View
5 appshell/appshell_extensions_platform.h
@@ -66,6 +66,11 @@ int32 ShowOpenDialog(bool allowMulitpleSelection,
ExtensionString fileTypes,
CefRefPtr<CefListValue>& selectedFiles);
+int32 ShowSaveDialog(ExtensionString title,
+ ExtensionString initialDirectory,
+ ExtensionString fileTypes,
+ ExtensionString& selectedFile);
+
int32 ReadDir(ExtensionString path, CefRefPtr<CefListValue>& directoryContents);
int32 MakeDir(ExtensionString path, int32 mode);
View
44 appshell/appshell_extensions_win.cpp
@@ -82,10 +82,10 @@ class LiveBrowserMgrWin
LiveBrowserMgrWin();
virtual ~LiveBrowserMgrWin();
- UINT m_closeLiveBrowserHeartbeatTimerId;
- UINT m_closeLiveBrowserTimeoutTimerId;
- CefRefPtr<CefProcessMessage> m_closeLiveBrowserCallback;
- CefRefPtr<CefBrowser> m_browser;
+ UINT m_closeLiveBrowserHeartbeatTimerId;
+ UINT m_closeLiveBrowserTimeoutTimerId;
+ CefRefPtr<CefProcessMessage> m_closeLiveBrowserCallback;
+ CefRefPtr<CefBrowser> m_browser;
static LiveBrowserMgrWin* s_instance;
};
@@ -510,6 +510,42 @@ int32 ShowOpenDialog(bool allowMultipleSelection,
return NO_ERROR;
}
+int32 ShowSaveDialog(ExtensionString title,
+ ExtensionString initialDirectory,
+ ExtensionString fileTypes,
+ ExtensionString& selectedFile)
+{
+ wchar_t* szFile = new wchar_t[MAX_PATH];
+
+ // ofn.lpstrInitialDir needs Windows path on XP and not Unix path.
+ ConvertToNativePath(initialDirectory);
+ wcscpy(szFile, initialDirectory.c_str());
+
+ OPENFILENAME ofn;
+
+ ZeroMemory(&ofn, sizeof(ofn));
+ ofn.hwndOwner = GetActiveWindow();
+ ofn.lStructSize = sizeof(ofn);
+ ofn.lpstrFile = szFile;
+ ofn.nMaxFile = MAX_PATH;
+
+ // TODO (issue #65) - Use passed in file types. Note, when fileTypesStr is null, all files should be shown
+ /* findAndReplaceString( fileTypesStr, std::string(" "), std::string(";*."));
+ LPCWSTR allFilesFilter = L"All Files\0*.*\0\0";*/
+
+ ofn.lpstrFilter = L"All Files\0*.*\0Web Files\0*.js;*.css;*.htm;*.html\0\0";
+
+ ofn.lpstrInitialDir = initialDirectory.c_str();
+ ofn.Flags = OFN_PATHMUSTEXIST | OFN_OVERWRITEPROMPT | OFN_NOCHANGEDIR | OFN_EXPLORER;
+
+ selectedFile = L"";
+ if (GetSaveFileName(&ofn)) {
+ selectedFile = szFile;
+ }
+
+ return NO_ERROR;
+}
+
int32 ReadDir(ExtensionString path, CefRefPtr<CefListValue>& directoryContents)
{
if (path.length() && path[path.length() - 1] != '/')
View
2  scripts/make_symlinks.bat
@@ -1,3 +1,5 @@
+@echo off
+
REM Remove existing links
rmdir Debug include lib libcef_dll Release tools
Something went wrong with that request. Please try again.