-
Notifications
You must be signed in to change notification settings - Fork 11.9k
refactor(config): refactor the config object. #1809
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import * as SilentError from 'silent-error'; | ||
| import * as Command from 'ember-cli/lib/models/command'; | ||
| import {CliConfig} from '../models/config'; | ||
|
|
||
|
|
@@ -11,10 +12,38 @@ const SetCommand = Command.extend({ | |
| { name: 'global', type: Boolean, default: false, aliases: ['g'] }, | ||
| ], | ||
|
|
||
| asBoolean: function (raw: string): boolean { | ||
| if (raw == 'true' || raw == '1') { | ||
| return true; | ||
| } else if (raw == 'false' || raw == '' || raw == '0') { | ||
| return false; | ||
| } else { | ||
| throw new SilentError(`Invalid boolean value: "${raw}"`); | ||
| } | ||
| }, | ||
| asNumber: function (raw: string): number { | ||
| if (Number.isNaN(+raw)) { | ||
| throw new SilentError(`Invalid number value: "${raw}"`); | ||
| } | ||
| return +raw; | ||
| }, | ||
|
|
||
| run: function (commandOptions, rawArgs): Promise<void> { | ||
| return new Promise(resolve => { | ||
| const config = new CliConfig(); | ||
| config.set(rawArgs[0], rawArgs[1], commandOptions.force); | ||
| const [jsonPath, rawValue] = rawArgs; | ||
| const config = CliConfig.fromProject(); | ||
| const type = config.typeOf(jsonPath); | ||
| let value: any = rawValue; | ||
|
|
||
| switch (type) { | ||
| case 'boolean': value = this.asBoolean(rawValue); break; | ||
| case 'number': value = this.asNumber(rawValue); break; | ||
| case 'string': value = rawValue; break; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this newline intended?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Basically between options and default. I can remove if you like.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need, just wanted to be sure it was intended. |
||
| default: value = JSON.parse(rawValue); | ||
| } | ||
|
|
||
| config.set(jsonPath, value); | ||
| config.save(); | ||
| resolve(); | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| import * as TestCommand from 'ember-cli/lib/commands/test'; | ||
| import * as config from '../models/config'; | ||
| import * as TestTask from '../tasks/test'; | ||
| import {CliConfig} from '../models/config'; | ||
|
|
||
| module.exports = TestCommand.extend({ | ||
| availableOptions: [ | ||
|
|
@@ -14,7 +14,7 @@ module.exports = TestCommand.extend({ | |
| ], | ||
|
|
||
| run: function (commandOptions) { | ||
| this.project.ngConfig = this.project.ngConfig || config.CliConfig.fromProject(); | ||
| this.project.ngConfig = this.project.ngConfig || CliConfig.fromProject(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually this is correct. We use
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But won't it break other commands if they are run in sequence? Everything else expects Or the other way around, if nothing else breaks by having one command assign the wrong thing to
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Weird, I thought I saw this being attributed loads of times.. but it's only in |
||
|
|
||
| var testTask = new TestTask({ | ||
| ui: this.ui, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rawValueshould just bevalue, no need forlet value = rawValue;in line 36.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rawValueis const, and of type string.valueis non-const, and of type any.