-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
resolves #739 add parent command as prefix of subcommand #740
Conversation
@@ -1031,9 +1031,12 @@ Command.prototype.helpInformation = function() { | |||
if (this._alias) { | |||
cmdName = cmdName + '|' + this._alias; | |||
} | |||
var ancestor = this | |||
var prefix = [] | |||
while ((ancestor = ancestor.parent)) prefix.push(ancestor.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.
Isn't it better to push just a name of ancestor to array (e.g. prefix.push(ancestor.name())
) and then...
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.
If this has multiple ancestors, e.g. git remote add
, this will add them in reverse order, so you get remote git add
var usage = [ | ||
'' | ||
,' Usage: ' + cmdName + ' ' + this.usage() | ||
,' Usage: ' + prefix.join('') + cmdName + ' ' + this.usage() |
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.
...join here by space (' '
) instead of empty string?
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.
The reason I did it this way is because it avoids adding an extra space if the list turns up empty. There are other ways to accomplish that of course, this was just a clever way to do it.
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.
[].join(' ') // => ''
Isn't it?
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.
Yes, but it's not the spaces between the arguments we're worried about, it's the trailing space. If the array is not empty, we want a trailing space. If the array is empty, we don't want a trailing space. That's why I add a trailing space to each entry. It accomplishes this outcome.
Again, it's just one way of getting the spaces correct. I'm happy to change it to something more clear if you don't feel comfortable with it.
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.
Something clearer might be:
const names = [];
for (const cmd = this; cmd; cmd = cmd.parent) {
names.unshift(cmd.name());
}
...
,' Usage: ' + names.join(' ') + ' ' + this.usage(),
Hi! When PR will be merged? Need this :) |
Thanks for questions from @vanesyan and @abetomo, replies from @mojavelinux, and comments from @simonbuchan. |
Adding link to #739 so shows up there. |
Merging into |
Closing in favour of #980 (which includes these commits) Thank you for your contributions. |
Available now as a prerelease. See #1001 |
No description provided.