-
Notifications
You must be signed in to change notification settings - Fork 12k
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
feature: New command 'set' and 'get' to play with the cli config. #407
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,6 +1,16 @@ | ||
node_modules/ | ||
.idea | ||
jsconfig.json | ||
npm-debug.log | ||
|
||
# IDEs | ||
.idea/ | ||
jsconfig.json | ||
|
||
# Typings file. | ||
typings/ | ||
|
||
# Misc | ||
tmp/ | ||
|
||
# Mac OSX Finder files. | ||
**/.DS_Store | ||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import * as chalk from 'chalk'; | ||
import * as Command from 'ember-cli/lib/models/command'; | ||
import {CliConfig} from '../models/config'; | ||
|
||
|
||
const GetCommand = Command.extend({ | ||
name: 'get', | ||
description: 'Set a value in the configuration.', | ||
works: 'outsideProject', | ||
|
||
availableOptions: [], | ||
|
||
run: function (commandOptions, rawArgs): Promise<void> { | ||
return new Promise(resolve => { | ||
const value = new CliConfig().get(rawArgs[0]); | ||
if (value === null) { | ||
console.error(chalk.red('Value cannot be found.')); | ||
} else if (typeof value == 'object') { | ||
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.
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.
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.
OOC, is thete any reason not to use |
||
console.log(JSON.stringify(value)); | ||
} else { | ||
console.log(value); | ||
} | ||
resolve(); | ||
}); | ||
} | ||
}); | ||
|
||
module.exports = GetCommand; | ||
module.exports.overrideCore = true; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import * as Command from 'ember-cli/lib/models/command'; | ||
import {CliConfig} from '../models/config'; | ||
|
||
|
||
const SetCommand = Command.extend({ | ||
name: 'set', | ||
description: 'Set a value in the configuration.', | ||
works: 'outsideProject', | ||
|
||
availableOptions: [ | ||
{ name: 'global', type: Boolean, default: false, aliases: ['g'] }, | ||
], | ||
|
||
run: function (commandOptions, rawArgs): Promise<void> { | ||
return new Promise(resolve => { | ||
const config = new CliConfig(); | ||
config.set(rawArgs[0], rawArgs[1], commandOptions.force); | ||
config.save(); | ||
resolve(); | ||
}); | ||
} | ||
}); | ||
|
||
module.exports = SetCommand; | ||
module.exports.overrideCore = true; | ||
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. You don't need to override this command as it doesn't exists in 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. Ah 👍 thanks. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,11 @@ module.exports = { | |
'version': require('./commands/version'), | ||
'completion': require('./commands/completion'), | ||
'doc': require('./commands/doc'), | ||
'github-pages-deploy': require('./commands/github-pages-deploy') | ||
'github-pages-deploy': require('./commands/github-pages-deploy'), | ||
|
||
// Configuration | ||
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. Do we need comment and break line here? 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. It's just to split it. We can do it, so why not do make it clearer? |
||
'set': require('./commands/set'), | ||
'get': require('./commands/get') | ||
}; | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,125 @@ | ||
import * as fs from 'fs'; | ||
import * as path from 'path'; | ||
|
||
|
||
export const CLI_CONFIG_FILE_NAME = 'angular-cli.json'; | ||
|
||
|
||
export interface CliConfigJson { | ||
routes?: { [name: string]: any }, | ||
packages?: { [name: string]: any } | ||
} | ||
|
||
|
||
function _findUp(name: string, from: string) { | ||
let currentDir = from; | ||
while (currentDir && currentDir != '/') { | ||
const p = path.join(currentDir, name); | ||
if (fs.existsSync(p)) { | ||
return p; | ||
} | ||
|
||
currentDir = path.resolve(currentDir, '..'); | ||
} | ||
|
||
return null; | ||
} | ||
|
||
|
||
export class CliConfig { | ||
constructor(private _config: CliConfigJson = CliConfig.fromProject()) {} | ||
|
||
save(path: string = CliConfig.configFilePath()) { | ||
if (!path) { | ||
throw new Error('Could not find config path.'); | ||
} | ||
|
||
fs.writeFileSync(path, JSON.stringify(this._config, null, 2), { encoding: 'utf-8' }); | ||
} | ||
|
||
set(jsonPath: string, value: any, force: boolean = false): boolean { | ||
let { parent, name, remaining } = this._findParent(jsonPath); | ||
while (force && remaining) { | ||
if (remaining.indexOf('.') != -1) { | ||
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.
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.
I don't think we should be super strict about 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. I suppose in TS it isn't as relevant, I'm just used to the strictness from normal JS. |
||
// Create an empty map. | ||
// TODO: create the object / array based on the Schema of the configuration. | ||
parent[name] = {}; | ||
} | ||
|
||
} | ||
|
||
parent[name] = value; | ||
return true; | ||
} | ||
|
||
get(jsonPath: string): any { | ||
let { parent, name, remaining } = this._findParent(jsonPath); | ||
if (remaining || !(name in parent)) { | ||
return null; | ||
} else { | ||
return parent[name]; | ||
} | ||
} | ||
|
||
private _validatePath(jsonPath: string) { | ||
if (!jsonPath.match(/^(?:[-_\w\d]+(?:\[\d+\])*\.)*(?:[-_\w\d]+(?:\[\d+\])*)$/)) { | ||
throw `Invalid JSON path: "${jsonPath}"`; | ||
} | ||
} | ||
|
||
private _findParent(jsonPath: string): { parent: any, name: string | number, remaining?: string } { | ||
this._validatePath(jsonPath); | ||
|
||
let parent: any = null; | ||
let current: any = this._config; | ||
|
||
const splitPath = jsonPath.split('.'); | ||
let name: string | number = ''; | ||
|
||
while (splitPath.length > 0) { | ||
const m = splitPath.shift().match(/^(.*?)(?:\[(\d+)\])*$/); | ||
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. What does 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. Nvm, I suppose it's match. 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. Yup. |
||
|
||
name = m[1]; | ||
const index: string = m[2]; | ||
parent = current; | ||
current = current[name]; | ||
|
||
if (current === null || current === undefined) { | ||
return { | ||
parent, | ||
name, | ||
remaining: (!isNaN(index) ? `[${index}]` : '') + splitPath.join('.') | ||
}; | ||
} | ||
|
||
if (!isNaN(index)) { | ||
name = index; | ||
parent = current; | ||
current = current[index]; | ||
|
||
if (current === null || current === undefined) { | ||
return { | ||
parent, | ||
name, | ||
remaining: splitPath.join('.') | ||
}; | ||
} | ||
} | ||
} | ||
|
||
return { parent, name }; | ||
} | ||
|
||
static configFilePath(projectPath?: string): string { | ||
// Find the configuration, either where specified, in the angular-cli project | ||
// (if it's in node_modules) or from the current process. | ||
return (projectPath && _findUp(CLI_CONFIG_FILE_NAME, projectPath)) | ||
|| _findUp(CLI_CONFIG_FILE_NAME, __dirname) | ||
|| _findUp(CLI_CONFIG_FILE_NAME, process.cwd()); | ||
} | ||
|
||
static fromProject(): CliConfigJson { | ||
const configPath = this.configFilePath(); | ||
return configPath ? require(configPath) : {}; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
# ngConfig - Design | ||
|
||
## Goals | ||
|
||
Currently, a project scaffolded with the CLI have no way of specifying options and configurations affecting their projects. There are ways to affect the build (with the `angular-cli-build.js` file), but the following questions cannot be answered without actual project options: | ||
|
||
* Where in my directory is my karma.conf file? | ||
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 this be separated questions? 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. It's just something that should be an option. |
||
* What is my firebase database URL? | ||
* Where is my client code? | ||
* How can I use a different lazy-loading boundary prefix (or none at all)? | ||
* Any other backend I want to run prior to `ng serve`? | ||
|
||
# Proposed Solution | ||
|
||
Since the data is static, we only need to keep it in a static store somewhere. | ||
|
||
One solution would be to keep the data in the `package.json`. Unfortunately, the metadata contains too much data and the `package.json` file would become unmanageable. | ||
|
||
Instead of polluting the package file, a `angular-cli.json` file will be created that contains all the values. Access to that file will be allowed to the user if he knows the structure of the file (unknown keys will be kept but ignored), and it's easy to read and write. We can also keep the file formatted. | ||
|
||
|
||
## Fallback | ||
|
||
There should be two `angular-cli.json` files; one for the project and a general one. The general one should contain information that can be useful when scaffolding new apps, or informations about the user. | ||
|
||
The project `angular-cli.json` goes into the project root. The global configuration should live at `$HOME/.angular-cli.json`. | ||
|
||
## Structure | ||
|
||
The structure should be defined by a JSON schema (see [here](http://json-schema.org/)). The schema will be used to generate the `d.ts`, but that file will be kept in the file system along the schema for IDEs. | ||
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. What is this 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. It lives next to the JSON schema. We will need both, unfortunately. 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. What is the 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. It contains the interface for the schema. Suppose a schema: {
"title": "Example Schema",
"type": "object",
"properties": {
"firstName": {
"type": "string"
},
"lastName": {
"type": "string"
},
"age": {
"description": "Age in years",
"type": "integer",
"minimum": 0
}
},
"required": ["firstName", "lastName"]
} We create an interface like: interface CliConfig {
firstName: string;
lastName: string;
age?: number;
} You can import that interface in TypeScript and the IDE will help you with autocorrect, completion, etc. 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. Ok, I think I get it. So this would help us in the CLI when importing the config directly. But that means there are cases when we should use the model to interact with the config, and cases where we should interact directly using this interface to help us, right? What scenarios should we not use the model? Also, I know it's not the real name but perhaps it shouldn't be |
||
|
||
Every PR that would change the schema should include the update to the `d.ts`. | ||
|
||
# API | ||
|
||
## CLI | ||
|
||
#### Getting values | ||
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. When getting values from within the CLI the options object should be exposed via the already existing project object. This will have the benefit of not having to require/import in the configuration object from many different places within the CLI. From commands/tasks/generate blueprints, I would like to see the ability to do the following:
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. Done. |
||
|
||
The new command `get` should be used to output values on the terminal. It takes a set of flags and an optional array of [paths](#path); | ||
|
||
* `--glob` or `-g`; the path follows a glob format, where `*` can be replaced by any amount of characters and `?` by a single character. This will output `name=value` for each values matched. | ||
|
||
Otherwise, outputs the value of the path passed in. If multiple paths are passed in, they follow the format of `name=value`. | ||
|
||
#### Setting values | ||
|
||
The new command `set` should be used to set values in the local configuration file. It takes a set of flags and an optional array of `[path](#path)=value`; | ||
|
||
* `--global`; sets the value in the global configuration. | ||
* `--remove`; removes the key (no value should be passed in). | ||
|
||
The schema needs to be taken into account when setting the value of the field; | ||
|
||
* If the field is a number, the string received from the command line is parsed. `NaN` throws an error. | ||
* If the field is an object, an error is thrown. | ||
* If the path is inside an object but the object hasn't been defined yet, sets the object with empty values (use the schema to create a valid object). | ||
|
||
#### Path<a name="path"></a> | ||
|
||
The paths are json formatted path; each `.` separates a map, while `[]` indicates an index in an array. | ||
|
||
An example is the following: | ||
|
||
keyA.keyB.arrayC[3].value | ||
|
||
## Model | ||
|
||
A model should be created that will include loading and saving the configuration, including the global configuration. | ||
|
||
**The model should be part of the project and created on the `project` object.** | ||
|
||
That model can be used internally by the tool to get information. It will include a proxy handler that throws if an operation doesn't respect the schema. It will also sets values on globals and locals depending on which branches you access. | ||
|
||
A simple API would return the TypeScript interface: | ||
|
||
```typescript | ||
class Config { | ||
// ... | ||
get local(): ICliConfig { /* ... */ } | ||
get global(): ICliConfig { /* ... */ } | ||
} | ||
``` | ||
|
||
The `local` and `global` getters return proxies that respect the JSON Schema defined for the Angular config. These proxies allow users to not worry about the existence of values; those values will only be created on disc when they are setted. | ||
|
||
Also, `local` will always defer to the same key-path in `global` if a value isn't available. If a value is set and the parent exists in `global`, it should be created to `local` such that it's saved locally to the project. The proxies only care about the end points of `local` and `global`, not the existence of a parent in either. | ||
|
||
For example, assuming the following globals/locals: | ||
|
||
```js | ||
// Global | ||
{ | ||
"key1": { | ||
"key2": { | ||
"value": 0, | ||
"value2": 1 | ||
} | ||
} | ||
} | ||
|
||
// Local | ||
{ | ||
"key1": { | ||
"key2": { | ||
"value2": 2, | ||
"value3": 3 | ||
} | ||
} | ||
} | ||
``` | ||
|
||
The following stands true: | ||
|
||
```typescript | ||
const config = new Config(/* ... */); | ||
|
||
console.log(config.local.key1.key2.value); // 0, even if it doesn't exist. | ||
console.log(config.local.key1.key2.value2); // 2, local overrides. | ||
console.log(config.local.key1.key2.value3); // 3. | ||
console.log(config.local.key1.key2.value4); // Schema's default value. | ||
|
||
console.log(config.global.key1.key2.value); // 0. | ||
console.log(config.global.key1.key2.value2); // 1, only global. | ||
console.log(config.global.key1.key2.value3); // Schema's default value. | ||
|
||
config.local.key1.key2.value = 1; | ||
// Now the value is 1 inside the local. Global stays the same. | ||
|
||
config.local.key1.key2.value3 = 5; | ||
// The global config isn't modified. | ||
|
||
config.global.key1.key2.value4 = 99; | ||
// The local config stays the same. | ||
console.log(config.local.key1.key2.value4); // 99, the global value. | ||
|
||
config.save(); // Commits if there's a change to global and/or local. | ||
``` | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
{ | ||
"compilerOptions": { | ||
"declaration": true, | ||
"emitDecoratorMetadata": true, | ||
"experimentalDecorators": true, | ||
"mapRoot": "", | ||
"module": "commonjs", | ||
"moduleResolution": "node", | ||
"noEmitOnError": true, | ||
"noImplicitAny": true, | ||
"outDir": "../dist/", | ||
"rootDir": ".", | ||
"sourceMap": true, | ||
"sourceRoot": "/", | ||
"target": "es5" | ||
} | ||
} | ||
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. We updated the 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. This is for the whole project, not the scaffolds :) |
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.
Shouldn't there be a
global
option ?