-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Update clasp completion #10377
base: master
Are you sure you want to change the base?
Update clasp completion #10377
Conversation
- list projects - list versions - list deployments
share/completions/clasp.fish
Outdated
@@ -1,104 +1,124 @@ | |||
function clasp_list_projects |
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.
__fish_clasp_list_projects
I am not sure whether it's the best approach to extract all JS function names for function __fish_clasp_list_functions
grep --extended-regexp '^\\s*function\\s+\\w+' --recursive --include '*.js' --no-filename --color=never . |
string replace --regex '\\s*function\\s+(\\w+).*' '$1'
end I guess the best solution is to utilize some JS parser to provide function names in a more reliable way (regex-based approach is not always accurate). This change also will allow us to provide some hints for function parameters like their default values. I have to do some research how to implement it, but it's my dream to do it. As a fallback the current way of extracting functions can be used. |
share/completions/clasp.fish
Outdated
string replace --regex '^\\s{2}([a-z]+).*' '$1' | ||
end | ||
|
||
function __fish_seen_subcommands_from |
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.
__fish_seen_subcommands_from
has a name clash.
If the generic one doesn't work for some reason, it's fine to pu it into clasp.fish
but use the __fish_clasp
prefix if you do that.
Else things will behave differently depending on which completion script happens to be loaded first.
share/completions/clasp.fish
Outdated
end | ||
|
||
function __fish_clasp_list_functions | ||
grep --extended-regexp '^\\s*function\\s+\\w+' --recursive --include '*.js' --no-filename --color=never . | |
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.
Some implementations of grep
don't support --include or --color.
It's probably not relevant to most users of clasp so I don't think this is blocking.. I think we can find a better solution anyway
I think it would be preferrable to add a command like clasp run --list-functions
to output the list of available functions because they will know best.
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.
Also long options are a compatibility issue in general, see e.g. the OpenBSD man page - which supports some long options, but only for compatibility with GNU grep where no short option is available.
share/completions/clasp.fish
Outdated
end | ||
|
||
function __fish_clasp_list_functions | ||
find . -name '*.js' -exec fish --command "cat {} | string replace --regex --filter '^\s*function\s+(\w+).*\$' '\$1'" \; |
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 will rerun one fish per file. Every single one of those fish processes will run config.fish.
That means not only may it not be idempotent, it can also be quite expensive. If you have 10 files and fish startup takes 100ms (which it very well might for people who use pyenv etc), this will take a second.
Either use sed
because you're already working on multiple files, or run something like
cat (find ...) | 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.
I've used sed
as it was suggested to increase the performance.
Description
Fixes issue #
TODOs: