Skip to content

[ENHANCEMENT] print double dash for long aliases in availableOptions - #5380

Merged
homu merged 1 commit into
ember-cli:masterfrom
ghedamat:print-command-long-aliases
Jan 22, 2016
Merged

homu merged 1 commit into
ember-cli:masterfrom
ghedamat:print-command-long-aliases

Conversation

@ghedamat

Copy link
Copy Markdown
Contributor

Presently if one alias for availableOptions is a long word the help function for that command will still print the option with a single dash although the -- version works as well.

i.e.

{ name: 'environment', type: String, default: 'development', aliases: ['e', { 'dev': 'development' }, { 'prod': 'production' }] },

will be printed as -dev even if also --dev is supported.

before this patch:

aliases: -a <value>, -long-a <value>, -b (--test-option=c), -long-b (--test-option=c)

after:

aliases: -a <value>, --long-a <value>, -b (--test-option=c), --long-b (--test-option=c)

@ghedamat

Copy link
Copy Markdown
Contributor Author

tests will fail as some existing options will be changed by this (in the printed help)

example test failure:

   --live-reload-port (Number) (Defaults to port number within [49152...65535])
-    aliases: --lrp <value>
+    aliases: -lrp <value>

before going in and changing the tests I'd like to know if the ember-cli team is ok with this change or if the current is intended behaviour and should be kept as is.

another option would be to consider printing -- for aliases longer than X chars (4 seems a reasonable value)

thanks!

@stefanpenner

Copy link
Copy Markdown
Contributor

This lgtm, @trabus / @rwjblue r+

@ghedamat

Copy link
Copy Markdown
Contributor Author

thanks @stefanpenner , if we set the limit to 4 chars instead of one the only help that would change are for the -in-repo and -inspr aliases

@rwjblue

rwjblue commented Jan 21, 2016

Copy link
Copy Markdown
Member

Ya, I agree that this is a good idea.

note: presently if one alias for the availableOptions is a long word
the help function for that command will still print the option with a single
dash although the `--` version seems to work fine

i.e.
https://github.com/ember-cli/ember-cli/blob/282641ba662292afb40cb88cdf31557a3e4cc6b7/lib%2Fcommands%2Fbuild.js#L13
will be printed as `-dev` even if also `--dev` is supported.

before:
aliases: -a <value>, -long-a <value>, -b (--test-option=c)

after:
aliases: -a <value>, --long-a <value>, -b (--test-option=c)
@ghedamat
ghedamat force-pushed the print-command-long-aliases branch from 5929f0c to d05fcd0 Compare January 21, 2016 16:42
@ghedamat

Copy link
Copy Markdown
Contributor Author

@stefanpenner @rwjblue appveyor is still running but I think we're good

I settled on a 4 char limit, let me know if that sounds good

thanks!

@trabus

trabus commented Jan 21, 2016

Copy link
Copy Markdown
Contributor

lgtm 👍

@stefanpenner

Copy link
Copy Markdown
Contributor

@homu r+

@homu

homu commented Jan 21, 2016

Copy link
Copy Markdown
Contributor

📌 Commit d05fcd0 has been approved by stefanpenner

homu added a commit that referenced this pull request Jan 21, 2016
…nner

[ENHANCEMENT] print double dash for long aliases in availableOptions

Presently if one alias for `availableOptions` is a long word the help function for that command will still print the option with a single dash although the `--` version works as well.

i.e.
https://github.com/ember-cli/ember-cli/blob/282641ba662292afb40cb88cdf31557a3e4cc6b7/lib%2Fcommands%2Fbuild.js#L13
will be printed as `-dev` even if also `--dev` is supported.

before this patch:
```
aliases: -a <value>, -long-a <value>, -b (--test-option=c), -long-b (--test-option=c)
```

after:
```
aliases: -a <value>, --long-a <value>, -b (--test-option=c), --long-b (--test-option=c)
```
@homu

homu commented Jan 21, 2016

Copy link
Copy Markdown
Contributor

⌛ Testing commit d05fcd0 with merge 9bade59...

@homu

homu commented Jan 21, 2016

Copy link
Copy Markdown
Contributor

💔 Test failed - status

@stefanpenner

Copy link
Copy Markdown
Contributor

@homu retry

@homu

homu commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

⌛ Testing commit d05fcd0 with merge a5371dd...

homu added a commit that referenced this pull request Jan 22, 2016
…nner

[ENHANCEMENT] print double dash for long aliases in availableOptions

Presently if one alias for `availableOptions` is a long word the help function for that command will still print the option with a single dash although the `--` version works as well.

i.e.
https://github.com/ember-cli/ember-cli/blob/282641ba662292afb40cb88cdf31557a3e4cc6b7/lib%2Fcommands%2Fbuild.js#L13
will be printed as `-dev` even if also `--dev` is supported.

before this patch:
```
aliases: -a <value>, -long-a <value>, -b (--test-option=c), -long-b (--test-option=c)
```

after:
```
aliases: -a <value>, --long-a <value>, -b (--test-option=c), --long-b (--test-option=c)
```
@homu

homu commented Jan 22, 2016

Copy link
Copy Markdown
Contributor

☀️ Test successful - status

@homu
homu merged commit d05fcd0 into ember-cli:master Jan 22, 2016
@ghedamat
ghedamat deleted the print-command-long-aliases branch January 22, 2016 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants