-
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
fix(@angular/cli): ng get: return whole config root when no path provided. #5887
Conversation
@@ -11,7 +11,7 @@ export interface GetOptions { | |||
|
|||
const GetCommand = Command.extend({ | |||
name: 'get', | |||
description: 'Get a value from the configuration.', | |||
description: 'Get a value from the configuration. Example: ng get project.name', |
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.
@prestonvanloon Please change this to match format eg. "ng new [name]".
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.
Done.
tests/e2e/tests/commands/get/get.ts
Outdated
@@ -4,6 +4,7 @@ import {expectToFail} from '../../../utils/utils'; | |||
export default function() { | |||
return Promise.resolve() | |||
.then(() => expectToFail(() => ng('get', 'apps.zzz.prefix'))) | |||
.then(() => expectToFail(() => ng('get', undefined))) |
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.
This should be just ng('get')
as ng get undefined
seems to be working fine already as it is converted to string key.
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.
Done.
I think this should be closed in favor of returning the whole config object when no param specified. I'll submit a new PR later. |
Or this PR modified to return the whole object. Not sure what is preferred. |
I updated this PR title, description, and content with the changes outlined in my previous comment. |
Heya @prestonvanloon thanks for the work here. Can you rebase this PR? There's a couple of CI errors that were due to changes on our side, but it should be fine now. |
@filipesilva OK. Should be all good now. Thanks! |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Running
ng get
without a config key would throw ainvalidJsonPath
error rather than a meaningful message.This PR
JSON.stringify(value, null, 2)
.Fixes #5886