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

Add completion for packages in update subcommand #4295

Conversation

Fuco1
Copy link
Contributor

@Fuco1 Fuco1 commented Aug 8, 2017

Description

This patch adds completion for the update subcommand, that is, when the user types in composer update <tab>.

The code depends on python for the json parsing. I'm not sure if this is appropriate or if there is a fish-native way to parse json data.

TODOs:

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

This patch adds completion for the update subcommand, that is, when the user types in `composer update <tab>`.

The code depends on python for the json parsing.  I'm not sure if this is appropriate or if there is a fish-native way to parse json data.
function __fish_composer_packages
echo "
import json
json_data = open('composer.json')
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it safe to assume composer.json is in the CWD?

Using python for this is fine since fish has no ability to handle JSON (this might change in the 3.x releases planned for next year).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The composer command only works when you are in the directory with composer.json. So nobody would usually start it from anywhere else most likely. But if they did the code above would probably fail.

I will add some error handling.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Fuco1, Ping? Just wondering if you're going to make this fail gracefully if the composer.json file isn't in the CWD. I don't think we want to merge this if it doesn't fail gracefully. Note that you don't have to modify the python code. You can simply predicate running the python code on whether the file exists using fish script. That is modify function __fish_composer_packages to do something like test -f composer.json; or return before running the python code. What we don't want are python exceptions being shown to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@krader1961 I totally forgot about this 😊 I'm a bit bummed this last week about some stuff, but I'm going to fix this right now so that I won't forget again!

@Fuco1
Copy link
Contributor Author

Fuco1 commented Aug 17, 2017

Well, so I've also added some missing commands and more suggestions for some other commands too. Maybe I should rename the pull request? We can also split it into more PRs if you want.

@krader1961
Copy link
Contributor

Squash merged as commit 3fe561b. Thx.

@krader1961 krader1961 closed this Aug 17, 2017
@krader1961 krader1961 added this to the fish 2.7.0 milestone Aug 17, 2017
@Fuco1
Copy link
Contributor Author

Fuco1 commented Aug 18, 2017

Awesome, thanks! :) Sorry for the delay.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants