Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from Jan 7, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 0 additions & 26 deletions atom/browser/api/atom_api_app.cc
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions atom/common/api/atom_api_command_line.cc
Expand Up @@ -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"

Expand All @@ -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
Expand Down
14 changes: 14 additions & 0 deletions docs/api/app.md
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is sad.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's how the chromium API behaves


### `app.enableSandbox()` _Experimental_ _macOS_ _Windows_

Enables full sandbox mode on the app.
Expand Down
23 changes: 11 additions & 12 deletions 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

Expand All @@ -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
miniak marked this conversation as resolved.
Show resolved Hide resolved
})
}

Object.assign(app, {
setApplicationMenu (menu) {
return Menu.setApplicationMenu(menu)
Expand All @@ -25,18 +32,10 @@ Object.assign(app, {
return Menu.getApplicationMenu()
},
commandLine: {
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))
}
})

Expand Down
70 changes: 70 additions & 0 deletions spec/api-app-spec.js
Expand Up @@ -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')
Expand Down Expand Up @@ -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)
}
})
15 changes: 15 additions & 0 deletions 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()
})
})
4 changes: 4 additions & 0 deletions spec/fixtures/api/command-line/package.json
@@ -0,0 +1,4 @@
{
"name": "command-line",
"main": "main.js"
}