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

Command to generate shell completion files #2077

Closed
tsloughter opened this issue May 13, 2019 · 12 comments · Fixed by #2858
Closed

Command to generate shell completion files #2077

tsloughter opened this issue May 13, 2019 · 12 comments · Fixed by #2858
Labels
enhancement new behaviour or additional functionality help wanted unlikely to be tackled by core maintainers

Comments

@tsloughter
Copy link
Collaborator

Instead of having hardcoded bash and zsh completion files we should generate them based on the providers configuration.

@tsloughter tsloughter added enhancement new behaviour or additional functionality help wanted unlikely to be tackled by core maintainers labels May 13, 2019
@MarkoMin
Copy link
Contributor

MarkoMin commented Jan 8, 2024

Now the results are out, here is my spawnfest project related to this issue. Notice that there are plenty of additional features alongside this, most important one is completion for plugins and their commands/flags. There are plenty of other features in this project, some implemented, some not.

I wonder if you want this to be part of rebar3 or should I keep it as a plugin?

@ferd
Copy link
Collaborator

ferd commented Jan 8, 2024

That looks pretty decent on the surface. I think it's the kind of stuff that could make sense within rebar3, which we could run as part of bootstrap or as a post-hook to escriptize at the very least.

Part of the challenge there is I think we'd need some tests to be able to know it doesn't break across releases. Do you have any idea of what tests could be added for it? I think spawnfest has none (which is totally understandable given the scope of a competition like that)

@MarkoMin
Copy link
Contributor

MarkoMin commented Jan 8, 2024

Well, testing it would be shell sorcery, but is doable I think.

To test a completion function itself, we can export COMP_WORDS and COMP_CWORD, then call _rebar3 and check if COMPREPLY contains all wanted replies. Try this (assuming you have current rebar3 autocomplete set):

export COMP_WORDS=("rebar3" "c")
export COMP_CWORD=1
_rebar3
echo ${COMPREPLY[0]}
echo ${COMPREPLY[1]}
...

COMPREPLY contains "clean", "compile", "cover" and "ct".

Apart from that, we'd need to test integration, i.e. make sure that the function gets called on when "rebar3" is the first word. That is possible by listing completions via complete -p and then check if there exists an entry whose last column contains "rebar3".

Everything said is ofc bash specific

@ferd
Copy link
Collaborator

ferd commented Jan 9, 2024

I think we could trust that the file format is an acceptable protocol to interact with shells, and that if we generate a well-formated file we're good. So the test could probably look for a generated file with the proper keywords in it that shows it has extracted the information we expect out of it.

@MarkoMin
Copy link
Contributor

MarkoMin commented Jan 9, 2024

What do you mean by "well-formated"? What I'd like to test is that

  1. completion will be triggered after "rebar3 " is on prompt
  2. completion file doesn't have syntax error
  3. it won't exit or stuck in a loop

Cost of false negatives and false positives is fairly low I'd say so we could skip testing those ATM.

@ferd
Copy link
Collaborator

ferd commented Jan 10, 2024

my point was that if:

  1. the completion file appears to have valid syntax
  2. it has the information we need (eg. commands and descriptions and arguments are found)

then we can trust that completion should be triggered and that it won't cause issues.

This can generally result in a simpler unit test that can run on any environment while still providing decent confidence that things work fine.

@MarkoMin
Copy link
Contributor

Yeah, that seems reasonable. But how to check the syntax if not by calling a function?

Apart from that, I wanted to a q about templates. So, when a new template is created, it expects key=val values as arguments. Is there any reason why those values can't be treated as flags to rebar3 new command. E.g. why not write rebar3 new app --name my_app --desc 'my desc' instead of rebar3 new app name=my_app desc='my_desc'. Maybe it's a nitpick, but I find kinda strange that only this command is expecting key=val values while others expect flags.

Also, is there a way to automatically integrate completion file on a trigger, e.g. after new plugin is fetched? I tried it with rebar_utils:sh/2 but with no success...

I'll try to come up with a PR in the following weeks.

@ferd
Copy link
Collaborator

ferd commented Jan 11, 2024

Re: templates, I think we just picked what was essentially the same syntax as the previous iteration of rebar (see https://github.com/rebar/rebar/blob/master/src/rebar_templater.erl) back in 2014, and never revisited, and now we're stuck with it if we want to remain compatible with ourselves.

I'd probably advise not to generate auto-completion for templates, because on top of the syntax that's messed up, that requires dynamically parsing global plugins and their files to deal with if you want it to be complete. It doesn't have to be complete, that being said.

There are hooks that exist, but we do not define default hooks. If we wanted to re-run autocompletion generation after each of these operations, we'd have to add commands for that by hand, and it's possibly sort of annoying to do that every run (including CI, or on OSes that don't use these shells in the first place). If you want to ignore it for now I'd be fine with that as well—by calling the command in a project manually as a user, all the [global and project-local] plugins required for it should be downloaded before the command is executed anyway.

@MarkoMin
Copy link
Contributor

and now we're stuck with it if we want to remain compatible with ourselves.

Would it worth to support both ways for backward compatibility, but deprecate the old way, eventually removing it in some further releases?

I'd probably advise not to generate auto-completion for templates, because on top of the syntax that's messed up, that requires dynamically parsing global plugins and their files to deal with if you want it to be complete. It doesn't have to be complete, that being said.

Agree, my implementation didn't use plugin templates, but would that be a hard thing to do? I have to use plugins whatsoever to get their commands and flags for completion. I'd love to have completions for templates because I think they are underused - they are used only to generate new projects, but not so often to generate files and I think that autocomplete would be a big step forward to it. Wouldn't be nice to write something like rebar3 new gen_server --name my_server --handle_info --code_change? Much more convenient than editor-based snippets. We could take it further and have behaviors-as-templates - rebar3 new my_behavior --optional_cb1 --optional_cb2 - creates a module with all exports, (maybe) specs and function signatures. But that is a complete new topic, maybe for the next spawnfest :D

and it's possibly sort of annoying to do that every run (including CI, or on OSes that don't use these shells in the first place)

Agree, but could there possibly be some kind of "when new plugin got compiled" trigger, not a trigger on every compile. That probably won't be achievable via hooks. I also agree about CI/OS stuff: we'd probably need a way to turn the mechanism off, probably via config (by default it is turned off - users who want it will probably turn it on in their global config).

@ferd
Copy link
Collaborator

ferd commented Jan 15, 2024

and now we're stuck with it if we want to remain compatible with ourselves.

Would it worth to support both ways for backward compatibility, but deprecate the old way, eventually removing it in some further releases?

I think our abstract plans for the future would be that the next major version is something fit to be included in Erlang/OTP and won't be a standalone release. Doing that shift would likely demand we move to argparse instead and require reworking how all command lines work for all providers and plugins.

That being said, supporting both formats would likely require reimplementing another parser, which we could probably do, but that we didn't really relish doing (we wanted to maintain less code, not more), and therefore never did.

Wouldn't be nice to write something like rebar3 new gen_server --name my_server --handle_info --code_change? Much more convenient than editor-based snippets. We could take it further and have behaviors-as-templates - rebar3 new my_behavior --optional_cb1 --optional_cb2 - creates a module with all exports, (maybe) specs and function signatures. But that is a complete new topic, maybe for the next spawnfest :D

Yeah. Whatever we do in the long term should probably plan to align with whatever format argparse requires. I think in general this is all alright, though currently, the way rebar3 loads the batches of possible arguments is based on the first initialization's values.

Specifically for templates though, a lot of arguments are acceptable, but only within the context of another one. Chances are that for templates, the thing that would work best there is to consider new to be a namespace, and for each template to be its own dynamic command -- a bit like we do for the results of alias. This would really enable per-template specifications down the road and to cut down on the ad-hoc implementations we have.

In fact, doing that (maybe with a fake namespace like rebar3 template <name>) might enable us to reuse the existing CLI parsing libraries, maintain the old functionality, and declare a brand new one as well that works better with auto-complete. So you'd have rebar3 template gen_server [list-of-args] and help for each of them be populated from whichever metadata file.

This is something we never did at first because it took us a few years to figure out we could dynamically load modules (which we did for alias). Now that we have that tool, we could drive it even further.

and it's possibly sort of annoying to do that every run (including CI, or on OSes that don't use these shells in the first place)

Agree, but could there possibly be some kind of "when new plugin got compiled" trigger, not a trigger on every compile. That probably won't be achievable via hooks. I also agree about CI/OS stuff: we'd probably need a way to turn the mechanism off, probably via config (by default it is turned off - users who want it will probably turn it on in their global config).

Yeah we don't really track runs and compilations that way, that would be a whole new mechanism. Also I'm realizing as I'm typing this that it might align with my previous proposal. We're clearly able to build plugins as we go and add them to the list of supported providers for tasks happening later in the build cycle. It's possible on the first run plugins wouldn't exist, but they would be added as they are made available to the execution flow by that mechanism.

@MarkoMin
Copy link
Contributor

I think our abstract plans for the future would be that the next major version is something fit to be included in Erlang/OTP and won't be a standalone release. Doing that shift would likely demand we move to argparse instead and require reworking how all command lines work for all providers and plugins.

That's the right path, but it will probably probably take a large amount of time (years?) to get there.

That being said, supporting both formats would likely require reimplementing another parser, which we could probably do, but that we didn't really relish doing (we wanted to maintain less code, not more), and therefore never did.

Indeed, but what if we use parser combinator to try the current parser and if it fails try the new one (argparse compatible). Then, when we decide to stop supporting the old key=val way, we just drop the current parser and instead of manually calling argparse:parse/2 in new provider, we just move argparse command to the provider opts and that's about it.

The thing is that I don't see a lot of code dependent on the new provider - affected code would probably be some ad-hoc init scripts to set projects up.

Chances are that for templates, the thing that would work best there is to consider new to be a namespace, and for each template to be its own dynamic command -- a bit like we do for the results of alias. This would really enable per-template specifications down the road and to cut down on the ad-hoc implementations we have.

Definitely agree! Also, if we move to argpase, we could technically write something like rebar3 new behavior gen_server ... where new is namespace, behavior is a provider and gen_server is argparse:command() which then contains its arguments.

In fact, doing that (maybe with a fake namespace like rebar3 template <name>) might enable us to reuse the existing CLI parsing libraries, maintain the old functionality, and declare a brand new one as well that works better with auto-complete. So you'd have rebar3 template gen_server [list-of-args] and help for each of them be populated from whichever metadata file.

That is also a valid option, but I'd suggest some other name, like create or something, because if we add behaviors here template might be confusing.

@ferd
Copy link
Collaborator

ferd commented Jan 16, 2024

I'd not move to argparse now at all, I think it's the long-term direction. Right now if we can reuse the existing parser we're minimizing cost of moving later, and that's probably good enough.

No issue on the name, I was just throwing random ones out there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement new behaviour or additional functionality help wanted unlikely to be tackled by core maintainers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants