-
Notifications
You must be signed in to change notification settings - Fork 33
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: update help text #608
Conversation
public getSetupEnvVar(shell: string): string { | ||
return `${this.cliBinEnvVar}_AC_${shell.toUpperCase()}_SETUP_PATH` | ||
} | ||
|
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.
I was building this string multiple times (and is also used in the output script) so made it a method in the base class.
src/commands/autocomplete/index.ts
Outdated
? `New-Item -Type Directory -Path (Split-Path -Parent $PROFILE) -ErrorAction SilentlyContinue | ||
Add-Content -Path $PROFILE -Value (Invoke-Expression -Command "${bin} autocomplete${this.config.topicSeparator}script ${shell}"); .$PROFILE` | ||
: `$ printf "eval $(${bin} autocomplete${this.config.topicSeparator}script ${shell})" >> ~/.${shell}rc; source ~/.${shell}rc` | ||
${chalk.cyan(`$ printf "eval $(${scriptCommand})" >> ~/.bashrc; source ~/.bashrc`)} |
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.
@jshackell-sfdc comparing this with the powershell
output where we don't include the $
sign, what do you think if we just remove it? then we don't need to tell users to copy the stuff after printf
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.
@cristiand391: yes, that's a good idea. And I assume you mean to remove ALL the "$", right? Including the ones before "sf "? In all shell outputs? That way it's consistent.
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.
correct, just left the $
in the examples shown in sf autocomplete --help
to be consistent with other commands.
Enjoy! | ||
|
||
`) | ||
expect(ctx.stdout).to.contain(`Setup Instructions for OCLIF-EXAMPLE CLI Autocomplete ---`) |
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.
instructions for all shells are wrapped in a single method so just made these tests look for the first line to ensure it's returning something.
This PR updates
autocomplete
cmd help text with improvements made by @jshackell-sfdc.No functional changes.
bash
zsh
powershell
@W-14810430@