feat: add app.commandLine.hasSwitch() / app.commandLine.getSwitchValue() (#16282)

* feat: add app.commandLine.hasSwitch() / app.commandLine.getSwitchValue()

* add more tests

* refactor: move appendSwitch / appendArgument to command_line module

* replace AppendSwitchASCII with AppendSwitchNative

* remove castArgs

atom/browser/api/atom_api_app.cc

@@ -30,7 +30,6 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/path_service.h"
-#include "base/strings/string_util.h"
 #include "base/sys_info.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/icon_manager.h"
@@ -48,7 +47,6 @@
 #include "native_mate/object_template_builder.h"
 #include "net/ssl/client_cert_identity.h"
 #include "net/ssl/ssl_cert_request_info.h"
-#include "services/network/public/cpp/network_switches.h"
 #include "services/service_manager/sandbox/switches.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/image/image.h"
@@ -1389,25 +1387,6 @@ void App::BuildPrototype(v8::Isolate* isolate,
 
 namespace {
 
-void AppendSwitch(const std::string& switch_string, mate::Arguments* args) {
-  auto* command_line = base::CommandLine::ForCurrentProcess();
-
-  if (base::EndsWith(switch_string, "-path",
-                     base::CompareCase::INSENSITIVE_ASCII) ||
-      switch_string == network::switches::kLogNetLog) {
-    base::FilePath path;
-    args->GetNext(&path);
-    command_line->AppendSwitchPath(switch_string, path);
-    return;
-  }
-
-  std::string value;
-  if (args->GetNext(&value))
-    command_line->AppendSwitchASCII(switch_string, value);
-  else
-    command_line->AppendSwitch(switch_string);
-}
-
 #if defined(OS_MACOSX)
 int DockBounce(const std::string& type) {
   int request_id = -1;
@@ -1428,14 +1407,9 @@ void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Context> context,
                 void* priv) {
   v8::Isolate* isolate = context->GetIsolate();
-  auto* command_line = base::CommandLine::ForCurrentProcess();
-
   mate::Dictionary dict(isolate, exports);
   dict.Set("App", atom::api::App::GetConstructor(isolate)->GetFunction());
   dict.Set("app", atom::api::App::Create(isolate));
-  dict.SetMethod("appendSwitch", &AppendSwitch);
-  dict.SetMethod("appendArgument", base::Bind(&base::CommandLine::AppendArg,
-                                              base::Unretained(command_line)));
 #if defined(OS_MACOSX)
   auto browser = base::Unretained(Browser::Get());
   dict.SetMethod("dockBounce", &DockBounce);

atom/common/api/atom_api_command_line.cc

@@ -2,10 +2,14 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
+#include "atom/common/native_mate_converters/file_path_converter.h"
 #include "atom/common/native_mate_converters/string16_converter.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
+#include "base/strings/string_util.h"
 #include "native_mate/converter.h"
 #include "native_mate/dictionary.h"
+#include "services/network/public/cpp/network_switches.h"
 
 #include "atom/common/node_includes.h"
 
@@ -20,13 +24,36 @@ base::CommandLine::StringType GetSwitchValue(const std::string& name) {
       name.c_str());
 }
 
+void AppendSwitch(const std::string& switch_string, mate::Arguments* args) {
+  auto* command_line = base::CommandLine::ForCurrentProcess();
+
+  if (base::EndsWith(switch_string, "-path",
+                     base::CompareCase::INSENSITIVE_ASCII) ||
+      switch_string == network::switches::kLogNetLog) {
+    base::FilePath path;
+    args->GetNext(&path);
+    command_line->AppendSwitchPath(switch_string, path);
+    return;
+  }
+
+  base::CommandLine::StringType value;
+  if (args->GetNext(&value))
+    command_line->AppendSwitchNative(switch_string, value);
+  else
+    command_line->AppendSwitch(switch_string);
+}
+
 void Initialize(v8::Local<v8::Object> exports,
                 v8::Local<v8::Value> unused,
                 v8::Local<v8::Context> context,
                 void* priv) {
+  auto* command_line = base::CommandLine::ForCurrentProcess();
   mate::Dictionary dict(context->GetIsolate(), exports);
   dict.SetMethod("hasSwitch", &HasSwitch);
   dict.SetMethod("getSwitchValue", &GetSwitchValue);
+  dict.SetMethod("appendSwitch", &AppendSwitch);
+  dict.SetMethod("appendArgument", base::Bind(&base::CommandLine::AppendArg,
+                                              base::Unretained(command_line)));
 }
 
 }  // namespace

docs/api/app.md

@@ -1159,6 +1159,20 @@ correctly.
 
 **Note:** This will not affect `process.argv`.
 
+### `app.commandLine.hasSwitch(switch)`
+
+* `switch` String - A command-line switch
+
+Returns `Boolean` - Whether the command-line switch is present.
+
+### `app.commandLine.getSwitchValue(switch)`
+
+* `switch` String - A command-line switch
+
+Returns `String` - The command-line switch value.
+
+**Note:** When the switch is not present, it returns empty string.
+
 ### `app.enableSandbox()` _Experimental_ _macOS_ _Windows_
 
 Enables full sandbox mode on the app.

lib/browser/api/app.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const bindings = process.atomBinding('app')
+const commandLine = process.atomBinding('command_line')
 const path = require('path')
 const { app, App } = bindings
 
@@ -25,18 +26,10 @@ Object.assign(app, {
     return Menu.getApplicationMenu()
   },
   commandLine: {
-    appendSwitch (...args) {
+    hasSwitch: (...args) => commandLine.hasSwitch(...args.map(String)),
-      const castedArgs = args.map((arg) => {
+    getSwitchValue: (...args) => commandLine.getSwitchValue(...args.map(String)),
-        return typeof arg !== 'string' ? `${arg}` : arg
+    appendSwitch: (...args) => commandLine.appendSwitch(...args.map(String)),
-      })
+    appendArgument: (...args) => commandLine.appendArgument(...args.map(String))
-      return bindings.appendSwitch(...castedArgs)
-    },
-    appendArgument (...args) {
-      const castedArgs = args.map((arg) => {
-        return typeof arg !== 'string' ? `${arg}` : arg
-      })
-      return bindings.appendArgument(...castedArgs)
-    }
   }
 })
 

spec/api-app-spec.js

@@ -6,6 +6,7 @@ const https = require('https')
 const net = require('net')
 const fs = require('fs')
 const path = require('path')
+const cp = require('child_process')
 const { ipcRenderer, remote } = require('electron')
 const { emittedOnce } = require('./events-helpers')
 const { closeWindow } = require('./window-helpers')
@@ -1091,4 +1092,73 @@ describe('app module', () => {
       return expect(app.whenReady()).to.be.eventually.fulfilled
     })
   })
+
+  describe('commandLine.hasSwitch', () => {
+    it('returns true when present', () => {
+      app.commandLine.appendSwitch('foobar1')
+      expect(app.commandLine.hasSwitch('foobar1')).to.be.true()
+    })
+
+    it('returns false when not present', () => {
+      expect(app.commandLine.hasSwitch('foobar2')).to.be.false()
+    })
+  })
+
+  describe('commandLine.hasSwitch (existing argv)', () => {
+    it('returns true when present', async () => {
+      const { hasSwitch } = await runCommandLineTestApp('--foobar')
+      expect(hasSwitch).to.be.true()
+    })
+
+    it('returns false when not present', async () => {
+      const { hasSwitch } = await runCommandLineTestApp()
+      expect(hasSwitch).to.be.false()
+    })
+  })
+
+  describe('commandLine.getSwitchValue', () => {
+    it('returns the value when present', () => {
+      app.commandLine.appendSwitch('foobar', 'æøåü')
+      expect(app.commandLine.getSwitchValue('foobar')).to.equal('æøåü')
+    })
+
+    it('returns an empty string when present without value', () => {
+      app.commandLine.appendSwitch('foobar1')
+      expect(app.commandLine.getSwitchValue('foobar1')).to.equal('')
+    })
+
+    it('returns an empty string when not present', () => {
+      expect(app.commandLine.getSwitchValue('foobar2')).to.equal('')
+    })
+  })
+
+  describe('commandLine.getSwitchValue (existing argv)', () => {
+    it('returns the value when present', async () => {
+      const { getSwitchValue } = await runCommandLineTestApp('--foobar=test')
+      expect(getSwitchValue).to.equal('test')
+    })
+
+    it('returns an empty string when present without value', async () => {
+      const { getSwitchValue } = await runCommandLineTestApp('--foobar')
+      expect(getSwitchValue).to.equal('')
+    })
+
+    it('returns an empty string when not present', async () => {
+      const { getSwitchValue } = await runCommandLineTestApp()
+      expect(getSwitchValue).to.equal('')
+    })
+  })
+
+  async function runCommandLineTestApp (...args) {
+    const appPath = path.join(__dirname, 'fixtures', 'api', 'command-line')
+    const electronPath = remote.getGlobal('process').execPath
+    const appProcess = cp.spawn(electronPath, [appPath, ...args])
+
+    let output = ''
+    appProcess.stdout.on('data', (data) => { output += data })
+
+    await emittedOnce(appProcess.stdout, 'end')
+
+    return JSON.parse(output)
+  }
 })

spec/fixtures/api/command-line/main.js

@@ -0,0 +1,15 @@
+const { app } = require('electron')
+
+app.on('ready', () => {
+  const payload = {
+    hasSwitch: app.commandLine.hasSwitch('foobar'),
+    getSwitchValue: app.commandLine.getSwitchValue('foobar')
+  }
+
+  process.stdout.write(JSON.stringify(payload))
+  process.stdout.end()
+
+  setImmediate(() => {
+    app.quit()
+  })
+})

spec/fixtures/api/command-line/package.json

@@ -0,0 +1,4 @@
+{
+  "name": "command-line",
+  "main": "main.js"
+}