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

Skip associative arguments that were already provided on `--prompt` #5015

Merged
merged 4 commits into from Dec 14, 2018

Conversation

2 participants
@schlessera
Copy link
Member

schlessera commented Dec 4, 2018

I'm ignoring positional arguments for now and concentrate on associative arguments, except the ones of type generic (--<field>=<value>).

The current PR seems to work as expected, however I'm not sure yet how to write a test for this functionality.

Fixes #4762

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Dec 12, 2018

As this is an interactive tool and not trivial to test, I'll merge this without test for now. It won't be used in scripting anyways, and users will probably provide feedback if it doesn't behave as it should.

@schlessera schlessera changed the title [WIP] Skip associative arguments that were already provided on `--prompt` Skip associative arguments that were already provided on `--prompt` Dec 12, 2018

@schlessera schlessera requested a review from wp-cli/committers Dec 12, 2018

@schlessera schlessera added this to the 2.1.0 milestone Dec 12, 2018

@danielbachhuber

This comment has been minimized.

Copy link
Member

danielbachhuber commented Dec 13, 2018

As this is an interactive tool and not trivial to test, I'll merge this without test for now.

I think it probably should have a test. There are some existing examples in https://github.com/wp-cli/wp-cli/blob/ed4b892dfad613b35ca54c62d4be3677a861f798/features/prompt.feature

Essentially, you can pipe a < SESSION file into STDIN to drive the interactive session.

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Dec 13, 2018

Yes, that's what I thought as well, but I couldn't get this to work across multiple arguments, which is needed here.

I'll have another go, maybe I messed it up somewhere along the way...

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Dec 13, 2018

😊 I found out why it didn't work... I forgot to actually pipe the file in the final call.

schlessera added some commits Dec 13, 2018

@schlessera

This comment has been minimized.

Copy link
Member

schlessera commented Dec 13, 2018

Ok, I added support for positional arguments and made the test work.

@danielbachhuber
Copy link
Member

danielbachhuber left a comment

Restarted the build

@schlessera schlessera merged commit cbc8a34 into master Dec 14, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@delete-merged-branch delete-merged-branch bot deleted the 4762-skip-provided-flags-on-prompt branch Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment