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 new option 'all' to output all matches #32

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add new option 'all' to output all matches #32

wants to merge 3 commits into from

Conversation

lamarpavel
Copy link

This commit adds the option '--all' / '-a', which will cause fzy to
ignore the selection and output all matching lines on return.

I needed this feature so I could call fzy from vim and fill a location-list with the output. I just found fzy today and was pleasantly surprised by how clean the code base is and how easy it was to add new functionality, so I just wrote this small patch. There might be better ways to solve this, possibly without changing fzy. Feel free to reject this PR and point me in a different direction in that case.

This commit adds the option '--all' / '-a', which will cause fzy to
ignore the selection and output all matching lines on return.
@insidewhy
Copy link

I've been using fzy for a day, it's really great! This feature is the only extra thing I really want from it.

src/options.c Outdated
@@ -10,6 +10,7 @@ static const char *usage_str =
""
"Usage: fzy [OPTION]...\n"
" -l, --lines=LINES Specify how many lines of results to show (default 10)\n"
" -a, --all Print all matched lines\n"

Choose a reason for hiding this comment

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

Indentation?

Copy link
Author

Choose a reason for hiding this comment

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

My mistake, vim inserted a tab and I didn't notice the other lines were spaces.

Choose a reason for hiding this comment

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

:set smarttab in your vim config.

printf("%s\n", state->search);
unsigned int all = state->options->output_all;
size_t start = all ? 0 : state->choices->selection;
size_t end = all ? choices_available(state->choices) : start + 1;

Choose a reason for hiding this comment

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

The rest of the code doesn't use whitespace to line up the = like that. Do you have an editor plugin that does it for you or do you have to do it manually? I'd be way too lazy for the latter :)

Copy link
Author

Choose a reason for hiding this comment

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

I use the easyalign-plugin for vim. The above alignment would be done in 3 keystrokes (after selecting the region of interest).

Choose a reason for hiding this comment

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

It sure is prettier, but I think it's bad. Say you add a fourth statement with a longer type and variable name, now you have to update 4 lines of code. If you just always use = then a single line of code won't involve changing others, keeping git diffs more readable. Yep, bad idea.

Choose a reason for hiding this comment

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

Plus having to select code and run the macro is also too much hassle, for it to be worth it (ignoring the source control issues) it would have to automatically detect consecutive assignments and update them all without needing any user selection.

Copy link
Author

Choose a reason for hiding this comment

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

Right. It's not important to me at all, I'd change that in a heart beat if it would stand in the way of the patch being accepted.

@insidewhy
Copy link

@jhawthorn would you be interested in this patch if it was cleaned up and submitted with a test? I'd really like to help get this feature delivered.

@insidewhy
Copy link

In #40 I added an action_emit_all which can be bound via the keybinding system added in that PR. This could be an alternative to the -a flag, or both could be supported with the -a flag swapping emit and emit all bindings.

@insidewhy
Copy link

@lamarpavel I'm going to maintain a fork that accepts contributions from the community at https://github.com/ohjames/fzu until the maintainer here has time to engage with us. It already has the ability to select "all matches" and configurable key bindings.

@lamarpavel
Copy link
Author

@ohjames Thank you for notifying me, and thanks to the AUR package I'm already using it.

@insidewhy
Copy link

Cool if you need a flag to make "select all" the default I'll happily accept it :) For me selecting with a binding is best.

@lamarpavel
Copy link
Author

That's all right, but maybe you should enable issues in your repo in case someone has comments to things not relevant for fzy (eg. you replaced one fzy -> fzu too many in the readme).

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.

None yet

2 participants