From 6c74e6b82a0150a9397d9ae3d753307b83290c76 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Fri, 4 Jan 2019 23:22:44 +0100 Subject: [PATCH 1/5] feat: add app.commandLine.hasSwitch() / app.commandLine.getSwitchValue() --- docs/api/app.md | 14 ++++++++++++++ lib/browser/api/app.js | 1 + spec/api-app-spec.js | 27 +++++++++++++++++++++++++++ 3 files changed, 42 insertions(+) diff --git a/docs/api/app.md b/docs/api/app.md index 3ba62c271253b..55dcd12d65db4 100644 --- a/docs/api/app.md +++ b/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. diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 297dcf1cc1a50..74077c17a9410 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -25,6 +25,7 @@ Object.assign(app, { return Menu.getApplicationMenu() }, commandLine: { + ...process.atomBinding('command_line'), appendSwitch (...args) { const castedArgs = args.map((arg) => { return typeof arg !== 'string' ? `${arg}` : arg diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index af4e48740257f..f0de08481d8ec 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -1091,4 +1091,31 @@ 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.getSwitchValue', () => { + it('returns the value when present', () => { + app.commandLine.appendSwitch('foobar', 'test') + expect(app.commandLine.getSwitchValue('foobar')).to.equal('test') + }) + + 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('') + }) + }) }) From cd5f1c4b249fe0f1f7a415b43dd78c55bad5151e Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Sat, 5 Jan 2019 10:25:51 +0100 Subject: [PATCH 2/5] add more tests --- spec/api-app-spec.js | 43 +++++++++++++++++++++ spec/fixtures/api/command-line/main.js | 15 +++++++ spec/fixtures/api/command-line/package.json | 4 ++ 3 files changed, 62 insertions(+) create mode 100644 spec/fixtures/api/command-line/main.js create mode 100644 spec/fixtures/api/command-line/package.json diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index f0de08481d8ec..e0ea8ab095b82 100644 --- a/spec/api-app-spec.js +++ b/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') @@ -1103,6 +1104,18 @@ describe('app module', () => { }) }) + 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', 'test') @@ -1118,4 +1131,34 @@ describe('app module', () => { 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) + } }) diff --git a/spec/fixtures/api/command-line/main.js b/spec/fixtures/api/command-line/main.js new file mode 100644 index 0000000000000..39e62cafbbc87 --- /dev/null +++ b/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() + }) +}) diff --git a/spec/fixtures/api/command-line/package.json b/spec/fixtures/api/command-line/package.json new file mode 100644 index 0000000000000..bbe8102015d01 --- /dev/null +++ b/spec/fixtures/api/command-line/package.json @@ -0,0 +1,4 @@ +{ + "name": "command-line", + "main": "main.js" +} From 848841eb81ee9f2173dd32d7b49ddb9827858e51 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 7 Jan 2019 03:57:25 +0100 Subject: [PATCH 3/5] refactor: move appendSwitch / appendArgument to command_line module --- atom/browser/api/atom_api_app.cc | 26 ----------------------- atom/common/api/atom_api_command_line.cc | 27 ++++++++++++++++++++++++ lib/browser/api/app.js | 24 ++++++++++----------- 3 files changed, 38 insertions(+), 39 deletions(-) diff --git a/atom/browser/api/atom_api_app.cc b/atom/browser/api/atom_api_app.cc index f9ae615149d9c..69c45d00bb682 100644 --- a/atom/browser/api/atom_api_app.cc +++ b/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 exports, v8::Local 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); diff --git a/atom/common/api/atom_api_command_line.cc b/atom/common/api/atom_api_command_line.cc index 24af1946f9031..dd85cf3a4df38 100644 --- a/atom/common/api/atom_api_command_line.cc +++ b/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; + } + + std::string value; + if (args->GetNext(&value)) + command_line->AppendSwitchASCII(switch_string, value); + else + command_line->AppendSwitch(switch_string); +} + void Initialize(v8::Local exports, v8::Local unused, v8::Local 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 diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 74077c17a9410..0ec4949277e90 100644 --- a/lib/browser/api/app.js +++ b/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 @@ -17,6 +18,12 @@ let dockMenu = null Object.setPrototypeOf(App.prototype, EventEmitter.prototype) EventEmitter.call(app) +const castArgs = function (args) { + return args.map((arg) => { + return typeof arg !== 'string' ? `${arg}` : arg + }) +} + Object.assign(app, { setApplicationMenu (menu) { return Menu.setApplicationMenu(menu) @@ -25,19 +32,10 @@ Object.assign(app, { return Menu.getApplicationMenu() }, commandLine: { - ...process.atomBinding('command_line'), - appendSwitch (...args) { - const castedArgs = args.map((arg) => { - return typeof arg !== 'string' ? `${arg}` : arg - }) - return bindings.appendSwitch(...castedArgs) - }, - appendArgument (...args) { - const castedArgs = args.map((arg) => { - return typeof arg !== 'string' ? `${arg}` : arg - }) - return bindings.appendArgument(...castedArgs) - } + hasSwitch: (...args) => commandLine.hasSwitch(...castArgs(args)), + getSwitchValue: (...args) => commandLine.getSwitchValue(...castArgs(args)), + appendSwitch: (...args) => commandLine.appendSwitch(...castArgs(args)), + appendArgument: (...args) => commandLine.appendArgument(...castArgs(args)) } }) From a20ad8e741974612939b0a78ee75858914dfd6a9 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 7 Jan 2019 04:18:13 +0100 Subject: [PATCH 4/5] replace AppendSwitchASCII with AppendSwitchNative --- atom/common/api/atom_api_command_line.cc | 4 ++-- spec/api-app-spec.js | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/atom/common/api/atom_api_command_line.cc b/atom/common/api/atom_api_command_line.cc index dd85cf3a4df38..953779a8dfadf 100644 --- a/atom/common/api/atom_api_command_line.cc +++ b/atom/common/api/atom_api_command_line.cc @@ -36,9 +36,9 @@ void AppendSwitch(const std::string& switch_string, mate::Arguments* args) { return; } - std::string value; + base::CommandLine::StringType value; if (args->GetNext(&value)) - command_line->AppendSwitchASCII(switch_string, value); + command_line->AppendSwitchNative(switch_string, value); else command_line->AppendSwitch(switch_string); } diff --git a/spec/api-app-spec.js b/spec/api-app-spec.js index e0ea8ab095b82..c709b3e42a023 100644 --- a/spec/api-app-spec.js +++ b/spec/api-app-spec.js @@ -1118,8 +1118,8 @@ describe('app module', () => { describe('commandLine.getSwitchValue', () => { it('returns the value when present', () => { - app.commandLine.appendSwitch('foobar', 'test') - expect(app.commandLine.getSwitchValue('foobar')).to.equal('test') + app.commandLine.appendSwitch('foobar', 'æøåü') + expect(app.commandLine.getSwitchValue('foobar')).to.equal('æøåü') }) it('returns an empty string when present without value', () => { From 9da0011804f405075617d3081bb2296a1fcfcf50 Mon Sep 17 00:00:00 2001 From: Milan Burda Date: Mon, 7 Jan 2019 13:47:55 +0100 Subject: [PATCH 5/5] remove castArgs --- lib/browser/api/app.js | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 0ec4949277e90..06ff5b5972363 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -18,12 +18,6 @@ let dockMenu = null Object.setPrototypeOf(App.prototype, EventEmitter.prototype) EventEmitter.call(app) -const castArgs = function (args) { - return args.map((arg) => { - return typeof arg !== 'string' ? `${arg}` : arg - }) -} - Object.assign(app, { setApplicationMenu (menu) { return Menu.setApplicationMenu(menu) @@ -32,10 +26,10 @@ Object.assign(app, { return Menu.getApplicationMenu() }, commandLine: { - hasSwitch: (...args) => commandLine.hasSwitch(...castArgs(args)), - getSwitchValue: (...args) => commandLine.getSwitchValue(...castArgs(args)), - appendSwitch: (...args) => commandLine.appendSwitch(...castArgs(args)), - appendArgument: (...args) => commandLine.appendArgument(...castArgs(args)) + hasSwitch: (...args) => commandLine.hasSwitch(...args.map(String)), + getSwitchValue: (...args) => commandLine.getSwitchValue(...args.map(String)), + appendSwitch: (...args) => commandLine.appendSwitch(...args.map(String)), + appendArgument: (...args) => commandLine.appendArgument(...args.map(String)) } })