-
Notifications
You must be signed in to change notification settings - Fork 280
Exposing "--all-input" option. #210
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
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could add an explanation that its helpful for things like git branches, mercurial bookmarks, maybe even directories -- who knows!
|
This is absolutely awesome! Really appreciate the thorough test coverage and cleanup along the way @gsheld. I'll take a deeper look tomorrow with some nits, but this can likely go in with minimal change. I agree that combining with a preconfigured command mode is nice. maybe provide a few example commands for git to isolate the bookmarks? for example, mercurial outputs something like: so if we matched on all text, you couldn't pipe it straight into also might have been nice to splice the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i know this would pass, but maybe add a test case with whitespace on both sides, just for completeness
input: ' foo bar ',
match: 'foo bar',
|
Alright more or less looked over everything -- some inline suggestions and nits that you can do in a followup (ideally), but this can go in! thanks again |
Exposing "--all-input" option.
|
@pcottle Glad you liked the diff. I'll create another PR for cleanup when I have a chance. I actually found a small bug with the commit you just merged as well, so that will also be addressed in the next PR. Thanks for reviewing! |
A new feature for
fppthat would allow the user to take full advantage of the input selection GUI without being constrained to input with file paths.For one small example, one could apply
fppto git branches:In "all input" mode, all lines are taken as valid input, assuming they contain more than just whitespace; each line is also trimmed of whitespace at its beginning and end.