Merge pull request #7182 from electron/shell-return-values

Return booleans from more shell APIs

atom/common/platform_util.h

@@ -21,11 +21,11 @@ namespace platform_util {
 
 // Show the given file in a file manager. If possible, select the file.
 // Must be called from the UI thread.
-void ShowItemInFolder(const base::FilePath& full_path);
+bool ShowItemInFolder(const base::FilePath& full_path);
 
 // Open the given file in the desktop's default manner.
 // Must be called from the UI thread.
-void OpenItem(const base::FilePath& full_path);
+bool OpenItem(const base::FilePath& full_path);
 
 // Open the given external protocol URL in the desktop's default manner.
 // (For example, mailto: URLs in the default mail user agent.)

atom/common/platform_util_linux.cc

@@ -59,16 +59,16 @@ namespace platform_util {
 // TODO(estade): It would be nice to be able to select the file in the file
 // manager, but that probably requires extending xdg-open. For now just
 // show the folder.
-void ShowItemInFolder(const base::FilePath& full_path) {
+bool ShowItemInFolder(const base::FilePath& full_path) {
   base::FilePath dir = full_path.DirName();
   if (!base::DirectoryExists(dir))
-    return;
+    return false;
 
-  XDGOpen(dir.value(), true);
+  return XDGOpen(dir.value(), true);
 }
 
-void OpenItem(const base::FilePath& full_path) {
-  XDGOpen(full_path.value(), true);
+bool OpenItem(const base::FilePath& full_path) {
+  return XDGOpen(full_path.value(), true);
 }
 
 bool OpenExternal(const GURL& url, bool activate) {

atom/common/platform_util_mac.mm

@@ -18,7 +18,7 @@
 
 namespace platform_util {
 
-void ShowItemInFolder(const base::FilePath& path) {
+bool ShowItemInFolder(const base::FilePath& path) {
   // The API only takes absolute path.
   base::FilePath full_path =
       path.IsAbsolute() ? path : base::MakeAbsoluteFilePath(path);
@@ -26,8 +26,11 @@ void ShowItemInFolder(const base::FilePath& path) {
   DCHECK([NSThread isMainThread]);
   NSString* path_string = base::SysUTF8ToNSString(full_path.value());
   if (!path_string || ![[NSWorkspace sharedWorkspace] selectFile:path_string
-                                        inFileViewerRootedAtPath:@""])
+                                        inFileViewerRootedAtPath:@""]) {
     LOG(WARNING) << "NSWorkspace failed to select file " << full_path.value();
+    return false;
+  }
+  return true;
 }
 
 // This function opens a file.  This doesn't use LaunchServices or NSWorkspace
@@ -37,11 +40,11 @@ void ShowItemInFolder(const base::FilePath& path) {
 //  2. Silent no-op for unassociated file types: http://crbug.com/50263
 // Instead, an AppleEvent is constructed to tell the Finder to open the
 // document.
-void OpenItem(const base::FilePath& full_path) {
+bool OpenItem(const base::FilePath& full_path) {
   DCHECK([NSThread isMainThread]);
   NSString* path_string = base::SysUTF8ToNSString(full_path.value());
   if (!path_string)
-    return;
+    return false;
 
   // Create the target of this AppleEvent, the Finder.
   base::mac::ScopedAEDesc<AEAddressDesc> address;
@@ -52,7 +55,7 @@ void OpenItem(const base::FilePath& full_path) {
                               address.OutPointer());  // result
   if (status != noErr) {
     OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE target";
-    return;
+    return false;
   }
 
   // Build the AppleEvent data structure that instructs Finder to open files.
@@ -65,7 +68,7 @@ void OpenItem(const base::FilePath& full_path) {
                               theEvent.OutPointer());  // result
   if (status != noErr) {
     OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE event";
-    return;
+    return false;
   }
 
   // Create the list of files (only ever one) to open.
@@ -76,7 +79,7 @@ void OpenItem(const base::FilePath& full_path) {
                         fileList.OutPointer());  // resultList
   if (status != noErr) {
     OSSTATUS_LOG(WARNING, status) << "Could not create OpenItem() AE file list";
-    return;
+    return false;
   }
 
   // Add the single path to the file list.  C-style cast to avoid both a
@@ -92,11 +95,11 @@ void OpenItem(const base::FilePath& full_path) {
     if (status != noErr) {
       OSSTATUS_LOG(WARNING, status)
           << "Could not add file path to AE list in OpenItem()";
-      return;
+      return false;
     }
   } else {
     LOG(WARNING) << "Could not get FSRef for path URL in OpenItem()";
-    return;
+    return false;
   }
 
   // Attach the file list to the AppleEvent.
@@ -106,7 +109,7 @@ void OpenItem(const base::FilePath& full_path) {
   if (status != noErr) {
     OSSTATUS_LOG(WARNING, status)
         << "Could not put the AE file list the path in OpenItem()";
-    return;
+    return false;
   }
 
   // Send the actual event.  Do not care about the reply.
@@ -122,6 +125,7 @@ void OpenItem(const base::FilePath& full_path) {
     OSSTATUS_LOG(WARNING, status)
         << "Could not send AE to Finder in OpenItem()";
   }
+  return status == noErr;
 }
 
 bool OpenExternal(const GURL& url, bool activate) {

atom/common/platform_util_win.cc

@@ -203,15 +203,15 @@ HRESULT DeleteFileProgressSink::ResumeTimer() {
 
 namespace platform_util {
 
-void ShowItemInFolder(const base::FilePath& full_path) {
+bool ShowItemInFolder(const base::FilePath& full_path) {
   base::win::ScopedCOMInitializer com_initializer;
   if (!com_initializer.succeeded())
-    return;
+    return false;
 
   base::FilePath dir = full_path.DirName().AsEndingWithSeparator();
   // ParseDisplayName will fail if the directory is "C:", it must be "C:\\".
   if (dir.empty())
-    return;
+    return false;
 
   typedef HRESULT (WINAPI *SHOpenFolderAndSelectItemsFuncPtr)(
       PCIDLIST_ABSOLUTE pidl_Folder,
@@ -232,29 +232,27 @@ void ShowItemInFolder(const base::FilePath& full_path) {
     HMODULE shell32_base = GetModuleHandle(L"shell32.dll");
     if (!shell32_base) {
       NOTREACHED() << " " << __FUNCTION__ << "(): Can't open shell32.dll";
-      return;
+      return false;
     }
     open_folder_and_select_itemsPtr =
         reinterpret_cast<SHOpenFolderAndSelectItemsFuncPtr>
             (GetProcAddress(shell32_base, "SHOpenFolderAndSelectItems"));
   }
   if (!open_folder_and_select_itemsPtr) {
-    ui::win::OpenFolderViaShell(dir);
-    return;
+    return ui::win::OpenFolderViaShell(dir);
   }
 
   base::win::ScopedComPtr<IShellFolder> desktop;
   HRESULT hr = SHGetDesktopFolder(desktop.Receive());
   if (FAILED(hr))
-    return;
+    return false;
 
   base::win::ScopedCoMem<ITEMIDLIST> dir_item;
   hr = desktop->ParseDisplayName(NULL, NULL,
                                  const_cast<wchar_t *>(dir.value().c_str()),
                                  NULL, &dir_item, NULL);
   if (FAILED(hr)) {
-    ui::win::OpenFolderViaShell(dir);
-    return;
+    return ui::win::OpenFolderViaShell(dir);
   }
 
   base::win::ScopedCoMem<ITEMIDLIST> file_item;
@@ -262,44 +260,43 @@ void ShowItemInFolder(const base::FilePath& full_path) {
       const_cast<wchar_t *>(full_path.value().c_str()),
       NULL, &file_item, NULL);
   if (FAILED(hr)) {
-    ui::win::OpenFolderViaShell(dir);
-    return;
+    return ui::win::OpenFolderViaShell(dir);
   }
 
   const ITEMIDLIST* highlight[] = { file_item };
 
   hr = (*open_folder_and_select_itemsPtr)(dir_item, arraysize(highlight),
                                           highlight, NULL);
-
-  if (FAILED(hr)) {
-    // On some systems, the above call mysteriously fails with "file not
-    // found" even though the file is there.  In these cases, ShellExecute()
-    // seems to work as a fallback (although it won't select the file).
-    if (hr == ERROR_FILE_NOT_FOUND) {
-      ui::win::OpenFolderViaShell(dir);
-    } else {
-      LPTSTR message = NULL;
-      DWORD message_length = FormatMessage(
-          FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
-          0, hr, 0, reinterpret_cast<LPTSTR>(&message), 0, NULL);
-      LOG(WARNING) << " " << __FUNCTION__
-                   << "(): Can't open full_path = \""
-                   << full_path.value() << "\""
-                   << " hr = " << hr
-                   << " " << reinterpret_cast<LPTSTR>(&message);
-      if (message)
-        LocalFree(message);
-
-      ui::win::OpenFolderViaShell(dir);
-    }
+  if (!FAILED(hr))
+    return true;
+
+  // On some systems, the above call mysteriously fails with "file not
+  // found" even though the file is there.  In these cases, ShellExecute()
+  // seems to work as a fallback (although it won't select the file).
+  if (hr == ERROR_FILE_NOT_FOUND) {
+    return ui::win::OpenFolderViaShell(dir);
+  } else {
+    LPTSTR message = NULL;
+    DWORD message_length = FormatMessage(
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM,
+        0, hr, 0, reinterpret_cast<LPTSTR>(&message), 0, NULL);
+    LOG(WARNING) << " " << __FUNCTION__
+                 << "(): Can't open full_path = \""
+                 << full_path.value() << "\""
+                 << " hr = " << hr
+                 << " " << reinterpret_cast<LPTSTR>(&message);
+    if (message)
+      LocalFree(message);
+
+    return ui::win::OpenFolderViaShell(dir);
   }
 }
 
-void OpenItem(const base::FilePath& full_path) {
+bool OpenItem(const base::FilePath& full_path) {
   if (base::DirectoryExists(full_path))
-    ui::win::OpenFolderViaShell(full_path);
+    return ui::win::OpenFolderViaShell(full_path);
   else
-    ui::win::OpenFileViaShell(full_path);
+    return ui::win::OpenFileViaShell(full_path);
 }
 
 bool OpenExternal(const base::string16& url, bool activate) {

docs/api/shell.md

@@ -20,13 +20,15 @@ The `shell` module has the following methods:
 
 * `fullPath` String
 
-Show the given file in a file manager. If possible, select the file.
+Show the given file in a file manager. If possible, select the file. Returns
+true if the item was successfully shown, false otherwise.
 
 ### `shell.openItem(fullPath)`
 
 * `fullPath` String
 
-Open the given file in the desktop's default manner.
+Open the given file in the desktop's default manner. Returns true if the item
+was successfully opened, false otherwise.
 
 ### `shell.openExternal(url[, options])`
 
@@ -44,6 +46,7 @@ application was available to open the URL, false otherwise.
 * `fullPath` String
 
 Move the given file to trash and returns a boolean status for the operation.
+Returns true if the item was successfully moved to the trash, false otherwise.
 
 ### `shell.beep()`