From 71dc30b6e699913bd2dd1f6b21d6245e4f798567 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
Date: Fri, 21 Dec 2018 09:58:35 -0700
Subject: [PATCH] feat: split openExternal into sync and async

---
 atom/common/api/atom_api_shell.cc | 47 +++++++++++++++++++++----------
 docs/api/shell.md                 | 20 +++++++++----
 script/prepare-release.js         |  4 +--
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/atom/common/api/atom_api_shell.cc b/atom/common/api/atom_api_shell.cc
index 7c67c7a3a7a64..368ca6908cc9d 100644
--- a/atom/common/api/atom_api_shell.cc
+++ b/atom/common/api/atom_api_shell.cc
@@ -10,6 +10,7 @@
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "atom/common/node_includes.h"
 #include "atom/common/platform_util.h"
+#include "atom/common/promise_util.h"
 #include "native_mate/dictionary.h"
 
 #if defined(OS_WIN)
@@ -43,17 +44,16 @@ struct Converter<base::win::ShortcutOperation> {
 
 namespace {
 
-void OnOpenExternalFinished(
-    v8::Isolate* isolate,
-    const base::Callback<void(v8::Local<v8::Value>)>& callback,
-    const std::string& error) {
+void OnOpenExternalFinished(v8::Isolate* isolate,
+                            scoped_refptr<atom::util::Promise> promise,
+                            const std::string& error) {
   if (error.empty())
-    callback.Run(v8::Null(isolate));
+    promise->Resolve();
   else
-    callback.Run(v8::String::NewFromUtf8(isolate, error.c_str()));
+    promise->RejectWithErrorMessage(error.c_str());
 }
 
-bool OpenExternal(
+bool OpenExternalSync(
 #if defined(OS_WIN)
     const base::string16& url,
 #else
@@ -69,17 +69,33 @@ bool OpenExternal(
     }
   }
 
-  if (args->Length() >= 3) {
-    base::Callback<void(v8::Local<v8::Value>)> callback;
-    if (args->GetNext(&callback)) {
-      platform_util::OpenExternal(
-          url, options,
-          base::Bind(&OnOpenExternalFinished, args->isolate(), callback));
-      return true;
+  return platform_util::OpenExternal(url, options);
+}
+
+v8::Local<v8::Promise> OpenExternal(
+#if defined(OS_WIN)
+    const base::string16& url,
+#else
+    const GURL& url,
+#endif
+    mate::Arguments* args) {
+  scoped_refptr<atom::util::Promise> promise =
+      new atom::util::Promise(args->isolate());
+
+  platform_util::OpenExternalOptions options;
+  if (args->Length() >= 2) {
+    mate::Dictionary obj;
+    if (args->GetNext(&obj)) {
+      obj.Get("activate", &options.activate);
+      obj.Get("workingDirectory", &options.working_dir);
     }
   }
 
-  return platform_util::OpenExternal(url, options);
+  platform_util::OpenExternal(
+      url, options,
+      base::Bind(&OnOpenExternalFinished, args->isolate(), promise));
+
+  return promise->GetHandle();
 }
 
 #if defined(OS_WIN)
@@ -144,6 +160,7 @@ void Initialize(v8::Local<v8::Object> exports,
   mate::Dictionary dict(context->GetIsolate(), exports);
   dict.SetMethod("showItemInFolder", &platform_util::ShowItemInFolder);
   dict.SetMethod("openItem", &platform_util::OpenItem);
+  dict.SetMethod("openExternalSync", &OpenExternalSync);
   dict.SetMethod("openExternal", &OpenExternal);
   dict.SetMethod("moveItemToTrash", &platform_util::MoveItemToTrash);
   dict.SetMethod("beep", &platform_util::Beep);
diff --git a/docs/api/shell.md b/docs/api/shell.md
index b38348ab6bdb4..cf3fbefb59299 100644
--- a/docs/api/shell.md
+++ b/docs/api/shell.md
@@ -34,21 +34,29 @@ Returns `Boolean` - Whether the item was successfully opened.
 
 Open the given file in the desktop's default manner.
 
-### `shell.openExternal(url[, options, callback])`
+### `shell.openExternalSync(url[, options])`
 
 * `url` String - Max 2081 characters on windows, or the function returns false.
 * `options` Object (optional)
   * `activate` Boolean (optional) - `true` to bring the opened application to the
     foreground. The default is `true`. _macOS_
   * `workingDirectory` String (optional) - The working directory. _Windows_
-* `callback` Function (optional) _macOS_ - If specified will perform the open asynchronously.
-  * `error` Error
 
 Returns `Boolean` - Whether an application was available to open the URL.
-If callback is specified, always returns true.
 
-Open the given external protocol URL in the desktop's default manner. (For
-example, mailto: URLs in the user's default mail agent).
+Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent).
+
+### `shell.openExternal(url[, options])`
+
+* `url` String - Max 2081 characters on windows, or the function returns false.
+* `options` Object (optional)
+  * `activate` Boolean (optional) - `true` to bring the opened application to the
+    foreground. The default is `true`. _macOS_
+  * `workingDirectory` String (optional) - The working directory. _Windows_
+
+Returns `Promise<Any>`
+
+Open the given external protocol URL in the desktop's default manner. (For example, mailto: URLs in the user's default mail agent).
 
 ### `shell.moveItemToTrash(fullPath)`
 
diff --git a/script/prepare-release.js b/script/prepare-release.js
index 13aa6e588fec6..5692f6ae26bd2 100755
--- a/script/prepare-release.js
+++ b/script/prepare-release.js
@@ -60,9 +60,7 @@ async function getReleaseNotes (currentBranch) {
   }
   console.log(`Generating release notes for ${currentBranch}.`)
   const releaseNotes = await releaseNotesGenerator(currentBranch)
-  if (releaseNotes.warning) {
-    console.warn(releaseNotes.warning)
-  }
+  if (releaseNotes.warning) console.warn(releaseNotes.warning)
   return releaseNotes
 }