Skip to content

CP-1122 Bash completions #96

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

Merged
merged 4 commits into from
Nov 20, 2015

Conversation

georgelesica-wf
Copy link
Contributor

Problem

People don't like typing, and dart_dev makes people type commands.

Solution

Add some Bash completions to dart_dev. I did these for myself and I thought maybe other people would want to use them as well.

They operate on the expectation that the ddev alias exists.

FYI

@evanweible-wf

@maxwellpeterson-wf
Copy link
Member

Cool! Any idea if this would work with zsh as well?

@georgelesica-wf
Copy link
Contributor Author

I spent a few minutes looking at the zsh completion documentation. It is pretty dense. I'm sure it could be done easily by someone with the requisite knowledge, however.

@georgelesica-wf
Copy link
Contributor Author

I'm going to add completions for subcommands as well. So please don't merge this yet. I should have mentioned that in the description. I just wanted to get it up here to get some feedback. If people are interested in zsh, I can definitely look into it. @bencampbell-wf is kind of a zsh wizard, so maybe he would be willing to put a few minutes into it as well.

@codecov-io
Copy link

Current coverage is 48.05%

Merging #96 into master will not affect coverage as of 3341216

Powered by Codecov. Updated on successful CI builds.

@evanweible-wf
Copy link
Contributor

This is awesome! I would also be interested in zsh completions 😃

@bencampbell-wf
Copy link
Contributor

It looks like the completion definitions for zsh are a cinch. The trick is installing them. While bash has a defacto user directory you can symlink your completion scripts into, zsh requires and update to $fpath before compinit runs.

I can take a crack at it if it's wanted.

@georgelesica-wf
Copy link
Contributor Author

OK, I'm pretty happy with the Bash completions at this point. @bencampbell-wf if you want to you can add zsh completions to this PR so we get them in all at once. Or we can do them separately.

@evanweible-wf
Copy link
Contributor

They operate on the expectation that the ddev alias exists.

@georgelesica-wf any way a check for this could be added so that we could display a meaningful error message if the alias doesn't exist?

@georgelesica-wf
Copy link
Contributor Author

@evanweible-wf oh there wouldn't be a problem if it doesn't exist, it just only does completion if you use ddev as the command.

@trentgrover-wf
Copy link
Contributor

+1

@evanweible-wf
Copy link
Contributor

Oh, duh 😄

+1

@trentgrover-wf
Copy link
Contributor

@jayudey-wf ready for merge

@bencampbell-wf if you get zsh magic together, please submit as a separate PR (I know I'd use it...)

@jayudey-wf jayudey-wf changed the title Bash completions CP-1122 Bash completions Nov 19, 2015
@georgelesica-wf
Copy link
Contributor Author

I realized I was making some assumptions about the environment people will have, so I added a bit more explanation to the README.

@georgelesica-wf
Copy link
Contributor Author

I'm not sure why the build is failing, I didn't change any code...

@evanweible-wf
Copy link
Contributor

It's an issue with Dart 1.13, I'm looking into it

@evanweible-wf
Copy link
Contributor

Also +1 to the readme updates.

@jayudey-wf
Copy link
Contributor

QA Resource Approval: +10

  • Testing instruction
  • Dev +1's
  • Dev/QA +10 with detail of what was tested
    • verified that completion works as expected on ddev tasks
  • Unit test created/updated
  • All unit tests pass

Merging into master.

jayudey-wf added a commit that referenced this pull request Nov 20, 2015
@jayudey-wf jayudey-wf merged commit cf73294 into Workiva:master Nov 20, 2015
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.

7 participants