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

[WIP] Add tab-completion for powershell and bash #2157

Merged
merged 2 commits into from
Nov 1, 2018

Conversation

kblohm
Copy link
Contributor

@kblohm kblohm commented Oct 14, 2018

Hi,
I tried to add some basic tab-completion for powershell.
This does probably not cover all the possibilities, but as far as i can tell it works fine for everything i tried.
Sometimes i feel like the fake-cli help is not super useful, so i might have missed some things.
One other painpoint is completion for targets. The --list argument needs to first download and build everything, so it might take a little while when he build never ran before.

I am not very firm in powershell-scripting, so i dont know if this is the right approach. I just looked how other people did something similair. Feedback is appreciated :).
There is no documentation yet, because i am not sure how useful this is or if you even want this in this repo. I could add some later though.

@matthid
Copy link
Member

matthid commented Oct 14, 2018

I try to not use powershell as much as possible, but I'm fine with adding this to this repository.

But we definitely would need a page to explain how people can use this. Also can we somehow ensure that if we add new CLI arguments we do not forget to update this powershell script?

@kblohm
Copy link
Contributor Author

kblohm commented Oct 14, 2018

I dont think there is an easy way to enforce that. The approach that the dotnet-cli is taking is kinda neat. The library that parses the commandline-arguments also creates suggestions, so the shell scripts only have to ask for them. (https:/dotnet/CliCommandLineParser)
That would require quite a lot of changes though. Especially calling the Target-Api from the script-runner.
It has the benefit of working for bash aswell.

So in short, i guess that is just discipline, like adding documentation for modules.
As far as i can tell, the bash-completion (if that is even used) is also not very up to date.

@BlythMeister
Copy link
Contributor

I'm unsure as to how many people actually call fake directly Vs using a script.
I don't think I've ever called fake directly... especially now that there is always a step to get fake at the right version into my repo.

@matthid
Copy link
Member

matthid commented Oct 14, 2018

@BlythMeister with fake 5 it makes much more sense now to use fake directly. Personally I always install globally and use that. Maybe we even add some fallback feature to use the local fake runtime version in the future

@BlythMeister
Copy link
Contributor

Hmm I guess I'm more concerned about repeatable builds because of legacy branching rather than trunk based.

@matthid
Copy link
Member

matthid commented Oct 14, 2018

@BlythMeister I guess I differentiate between CI and local builds. Locally it is nice to just fake build in the shell, on CI it will use always the same fake-runtime version. But I don't really want to steal this issue, which is about powershell completion. We can discuss this in another issue if needed.

@matthid matthid added the waiting for author Some information or action was requested and it needs to be addressed. Or a response from author label Oct 18, 2018
@kblohm kblohm force-pushed the posh-fake branch 2 times, most recently from 7a9254e to ce4ac5b Compare October 20, 2018 14:39
@matthid
Copy link
Member

matthid commented Oct 25, 2018

Is this ready? or rather what is missing?

@matthid matthid removed the waiting for author Some information or action was requested and it needs to be addressed. Or a response from author label Oct 25, 2018
@kblohm
Copy link
Contributor Author

kblohm commented Oct 26, 2018

Sorry, i forgot to write a comment after i pushed.
I wanted to add a bash-version. But i can also make another PR for that if you want to merge.

- add completion for commands, options and targets in bash
- update documentation for command-line-completion to include bash
- update installation instructions for completions
@kblohm
Copy link
Contributor Author

kblohm commented Oct 28, 2018

I added a completion file for bash. I could only test it in git-bash though. It would be nice if someone could have another look. I am also not 100% sure if the installation instructions work on every system, because i could not test them :/.

@kblohm kblohm changed the title [WIP] Add tab-completion for powershell [WIP] Add tab-completion for powershell and bash Oct 28, 2018
@matthid
Copy link
Member

matthid commented Nov 1, 2018

Just installed the bash version and it looks quite good. There is no completion for targets and
fake build completes to fake build target, while you might also add arguments, similarly
fake build --e completes to fake build --environment-variable and one more tab adds fake build --environment-variable target which is not actually a valid command line (well, it is "valid" and runs but probably not what the user was trying to do and the argument is ignored).

Nevertheless I think it is already quite good so we can fix those issues as we go forward.

@matthid matthid merged commit 0ccf701 into fsprojects:release/next Nov 1, 2018
@kblohm
Copy link
Contributor Author

kblohm commented Nov 1, 2018

I had a look at the git completion where you also have to write -- to complete arguments. Otherwise it will default to commands e.g add, commit etc. I am not sure what the UNIX default is there. When i write fake build target and double tab i get a list of targets. That list is somewhat wrong though :/. I forgot to handle targets with descriptions.

@matthid
Copy link
Member

matthid commented Nov 1, 2018

@kblohm Yes sorry just tested it again and it works, I just was in another directory and then you just get the default completion (list of files)

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.

3 participants