search

ibhagwan / fzf-lua

Improved fzf.vim written in lua
MIT License
2.38k stars 151 forks source link

feat(keymaps): add the `show_details` option #1452

Closed loyd closed 2 months ago

loyd commented 2 months ago

Hi, thanks for this package!

I'm migrating from the telescope (because of slowness on large codebases), and I noticed that the keymaps picker is too restrictive, which is not very useful for those who provide detailed descriptions for all bindings: 2024-09-22_00-13-34

This PR introduces the show_details option, which (if false) disables the detail column completely and shows this info only if description is empty.

By default, show_details = true, thus it doesn't affect existing users.

Result: 2024-09-22_00-13-53

ibhagwan commented 2 months ago

Ty for the PR @loyd, I answered your Q in the review and a minor comment.

Did you check the preview of the lua function callback works without show_detsil? I’m thinking perhaps we can have a more flexible approach to the issue reordering the entries with --with-nth?

We can set --delimiter=|, place the description last (with no substring limitation) and then use --with-nth=1,2,4,3 for a “show_detail” entry and --with-nth=1,2,4 without (the 3rd field being the details).

loyd commented 2 months ago

Ty for the PR @loyd, I answered your Q in the review and a minor comment.

Thanks for the fast review. Fixed.

Did you check the preview of the lua function callback works without show_detsil?

Yep, of course, it looks as previously, but without an extra three spaces in the description column.

We can set --delimiter=|, place the description last (with no substring limitation) and then use --with-nth=1,2,4,3 for a “show_detail” entry and --with-nth=1,2,4 without (the 3rd field being the details).

That would be nice. However, it means columns cannot include | (unlikely, but possible).

ibhagwan commented 2 months ago

That would be nice. However, it means columns cannot include | (unlikely, but possible).

you’re right, perhaps we can add utils.nbsp .. | as the column separator and use utils.nbsp as delimiter?

loyd commented 2 months ago

use --with-nth=1,2,4,3 for a “show_detail” and --with-nth=1,2,4 without

The column shouldn't be limited, but I doubt limiting detail is a good idea for users without good descriptions.

Thus, details should be limited (and with %-33s) if show_details=true and shouldn't be otherwise. That means it's not only about ordering, and --with-nth is not enough. So, I'm not sure it's significant improvements in code (I've tried it).

Maybe calculate the real width of columns?

ibhagwan commented 2 months ago

use --with-nth=1,2,4,3 for a “show_detail” and --with-nth=1,2,4 without

The column shouldn't be limited, but I doubt limiting detail is a good idea for users without good descriptions.

Thus, details should be limited (and with %-33s) if show_details=true and shouldn't be otherwise. That means it's not only about ordering, and --with-nth is not enough. So, I'm not sure it's significant improvements in code (I've tried it).

Maybe calculate the real width of columns?

Maybe I’m overthinking this, show_detail option the way you did it should be perfect :)

ibhagwan commented 2 months ago

And ty so much for the contribution @loyd :)