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

Add quiet option #541

Merged
merged 3 commits into from
Dec 9, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Added

* `quiet` option (#541)

## [8.3.0] - 2016-11-16

### Changed
Expand Down
32 changes: 24 additions & 8 deletions common.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ function parseCLIArgs (argv) {
'deref-symlinks',
'download.strictSSL',
'overwrite',
'prune'
'prune',
'quiet'
],
default: {
'deref-symlinks': true,
Expand Down Expand Up @@ -99,9 +100,21 @@ function generateFinalPath (opts) {
return path.join(opts.out || process.cwd(), generateFinalBasename(opts))
}

function subOptionWarning (properties, optionName, parameter, value) {
function info (message, quiet) {
if (!quiet) {
console.error(message)
Copy link

@MatthewTeolis MatthewTeolis Aug 15, 2017

Choose a reason for hiding this comment

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

@malept Is there a reason you chose console.error instead of console.info/console.log?

When running in Maven and Jenkins, console.error creates a misleading error message (also displayed in red):

[ERROR] Packaging app for platform win32 x64 using electron v1.6.10

This makes it look like the packager failed to package the app

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't. Explanation is here: #228 (comment)

}
}

function warning (message, quiet) {
if (!quiet) {
console.warn(message)
}
}

function subOptionWarning (properties, optionName, parameter, value, quiet) {
if (properties.hasOwnProperty(parameter)) {
console.warn(`WARNING: ${optionName}.${parameter} will be inferred from the main options`)
warning(`WARNING: ${optionName}.${parameter} will be inferred from the main options`, quiet)
}
properties[parameter] = value
}
Expand All @@ -115,7 +128,7 @@ function createAsarOpts (opts) {
} else if (opts.asar === false || opts.asar === undefined) {
return false
} else {
console.warn(`asar parameter set to an invalid value (${opts.asar}), ignoring and disabling asar`)
warning(`asar parameter set to an invalid value (${opts.asar}), ignoring and disabling asar`)
return false
}

Expand All @@ -125,9 +138,9 @@ function createAsarOpts (opts) {
function createDownloadOpts (opts, platform, arch) {
let downloadOpts = Object.assign({}, opts.download)

subOptionWarning(downloadOpts, 'download', 'platform', platform)
subOptionWarning(downloadOpts, 'download', 'arch', arch)
subOptionWarning(downloadOpts, 'download', 'version', opts.version)
subOptionWarning(downloadOpts, 'download', 'platform', platform, opts.quiet)
subOptionWarning(downloadOpts, 'download', 'arch', arch, opts.quiet)
subOptionWarning(downloadOpts, 'download', 'version', opts.version, opts.quiet)

return downloadOpts
}
Expand Down Expand Up @@ -173,6 +186,8 @@ module.exports = {
generateFinalBasename: generateFinalBasename,
generateFinalPath: generateFinalPath,

info: info,

initializeApp: function initializeApp (opts, templatePath, appRelativePath, callback) {
// Performs the following initial operations for an app:
// * Creates temporary directory
Expand Down Expand Up @@ -284,5 +299,6 @@ module.exports = {
debug(`Renaming ${oldName} to ${newName} in ${basePath}`)
fs.rename(path.join(basePath, oldName), path.join(basePath, newName), cb)
},
sanitizeAppName: sanitizeAppName
sanitizeAppName: sanitizeAppName,
warning: warning
}
7 changes: 7 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,13 @@ The non-`all` values correspond to the platform names used by [Electron releases

Runs [`npm prune --production`](https://docs.npmjs.com/cli/prune) before starting to package the app.

##### `quiet`

*Boolean* (default: `false`)

If `true`, disables printing informational and warning messages to the console when packaging the
application. This does *not* disable errors.

##### `tmpdir`

*String* or *`false`* (default: system temp directory)
Expand Down
6 changes: 3 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ function createSeries (opts, archs, platforms) {
buildParentDir = opts.out || process.cwd()
}
var buildDir = path.join(buildParentDir, `${platform}-${arch}-template`)
console.error(`Packaging app for platform ${platform} ${arch} using electron v${version}`)
common.info(`Packaging app for platform ${platform} ${arch} using electron v${version}`, opts.quiet)
series([
function (cb) {
debug(`Creating ${buildDir}`)
Expand Down Expand Up @@ -203,7 +203,7 @@ function createSeries (opts, archs, platforms) {
createApp(comboOpts)
})
} else {
console.error(`Skipping ${platform} ${arch} (output dir already exists, use --overwrite to force)`)
common.info(`Skipping ${platform} ${arch} (output dir already exists, use --overwrite to force)`, opts.quiet)
callback()
}
} else {
Expand All @@ -216,7 +216,7 @@ function createSeries (opts, archs, platforms) {
testSymlink(function (result) {
if (result) return checkOverwrite()

console.error(`Cannot create symlinks (on Windows hosts, it requires admin privileges); skipping ${combination.platform} platform`)
common.info(`Cannot create symlinks (on Windows hosts, it requires admin privileges); skipping ${combination.platform} platform`, opts.quiet)
callback()
})
} else {
Expand Down
18 changes: 9 additions & 9 deletions mac.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,18 +199,18 @@ class MacApp {

if ((platform === 'all' || platform === 'mas') &&
osxSignOpt === undefined) {
console.warn('WARNING: signing is required for mas builds. Provide the osx-sign option, ' +
'or manually sign the app later.')
common.warning('WARNING: signing is required for mas builds. Provide the osx-sign option, ' +
'or manually sign the app later.')
}

if (osxSignOpt) {
this.operations.push((cb) => {
let signOpts = createSignOpts(osxSignOpt, platform, this.renamedAppPath, version)
let signOpts = createSignOpts(osxSignOpt, platform, this.renamedAppPath, version, this.opts.quiet)
debug(`Running electron-osx-sign with the options ${JSON.stringify(signOpts)}`)
sign(signOpts, (err) => {
if (err) {
// Although not signed successfully, the application is packed.
console.warn('Code sign failed; please retry manually.', err)
common.warning('Code sign failed; please retry manually.', err)
}
cb()
})
Expand All @@ -233,19 +233,19 @@ function filterCFBundleIdentifier (identifier) {
return identifier.replace(/ /g, '-').replace(/[^a-zA-Z0-9.-]/g, '')
}

function createSignOpts (properties, platform, app, version) {
function createSignOpts (properties, platform, app, version, quiet) {
// use default sign opts if osx-sign is true, otherwise clone osx-sign object
let signOpts = properties === true ? {identity: null} : Object.assign({}, properties)

// osx-sign options are handed off to sign module, but
// with a few additions from the main options
// user may think they can pass platform, app, or version, but they will be ignored
common.subOptionWarning(signOpts, 'osx-sign', 'platform', platform)
common.subOptionWarning(signOpts, 'osx-sign', 'app', app)
common.subOptionWarning(signOpts, 'osx-sign', 'version', version)
common.subOptionWarning(signOpts, 'osx-sign', 'platform', platform, quiet)
common.subOptionWarning(signOpts, 'osx-sign', 'app', app, quiet)
common.subOptionWarning(signOpts, 'osx-sign', 'version', version, quiet)

if (signOpts.binaries) {
console.warn('WARNING: osx-sign.binaries is not an allowed sub-option. Not passing to electron-osx-sign.')
common.warning('WARNING: osx-sign.binaries is not an allowed sub-option. Not passing to electron-osx-sign.')
delete signOpts.binaries
}

Expand Down
17 changes: 17 additions & 0 deletions test/basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,23 @@ test('validateListFromOptions does not take non-Array/String values', (t) => {
t.end()
})

test('setting the quiet option does not print messages', (t) => {
const errorLog = console.error
const warningLog = console.warn
let output = ''
console.error = (message) => { output += message }
console.warn = (message) => { output += message }

common.warning('warning', true)
t.equal('', output, 'quieted common.warning should not call console.warn')
common.info('info', true)
t.equal('', output, 'quieted common.info should not call console.error')

console.error = errorLog
console.warn = warningLog
t.end()
})

test('download argument test: download.{arch,platform,version} does not overwrite {arch,platform,version}', function (t) {
var opts = {
download: {
Expand Down
1 change: 1 addition & 0 deletions usage.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ overwrite if output directory for a platform already exists, replaces i
skipping it
platform all, or one or more of: darwin, linux, mas, win32 (comma-delimited if multiple).
Defaults to the host platform
quiet Do not print informational or warning messages
tmpdir temp directory. Defaults to system temp directory, use --tmpdir=false to disable
use of a temporary directory.
version the version of Electron that is being packaged, see
Expand Down