Skip to content
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

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

EmilyGraceSeville7cf
Copy link
Contributor

@EmilyGraceSeville7cf EmilyGraceSeville7cf commented Mar 16, 2024

Description

  • refactor code
  • provide completions for projects, versions, deployments, advanced services and .js functions

Fixes issue #

TODOs:

  • Changes to fish usage are reflected in user documentation/manpages.
  • Tests have been added for regressions fixed
  • User-visible changes noted in CHANGELOG.rst

- list projects
- list versions
- list deployments
@@ -1,104 +1,124 @@
function clasp_list_projects
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__fish_clasp_list_projects

@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as draft March 18, 2024 14:03
@EmilyGraceSeville7cf EmilyGraceSeville7cf marked this pull request as ready for review March 18, 2024 15:09
@EmilyGraceSeville7cf
Copy link
Contributor Author

EmilyGraceSeville7cf commented Mar 18, 2024

I am not sure whether it's the best approach to extract all JS function names for clasp run {{function}} {{argument...}} command:

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.

string replace --regex '^\\s{2}([a-z]+).*' '$1'
end

function __fish_seen_subcommands_from
Copy link
Member

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.

end

function __fish_clasp_list_functions
grep --extended-regexp '^\\s*function\\s+\\w+' --recursive --include '*.js' --no-filename --color=never . |
Copy link
Member

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.

Copy link
Member

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.

end

function __fish_clasp_list_functions
find . -name '*.js' -exec fish --command "cat {} | string replace --regex --filter '^\s*function\s+(\w+).*\$' '\$1'" \;
Copy link
Member

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

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.

None yet

3 participants