feat: add finder option for choosing a preferred finder backend (#129)
#130
Conversation
idanarye
force-pushed
the
allow-configuring-the-finder-in-the-plugin-setup
branch
2 times, most recently
from
5404f5a
to
7983737
Compare
| end | ||
| choose_template() | ||
| return true | ||
| end |
There was a problem hiding this comment.
I think we should explicitly return false if has_telescope is false, and same for the others below. Just for clarity.
There was a problem hiding this comment.
Should I also verify in run_first_supported that the returned value is boolean?
There was a problem hiding this comment.
Another option is to throw a magic value from skipped implementations. That way run_first_supported can return the value of the implementation that succeeded.
There was a problem hiding this comment.
Another option is to throw a magic value from skipped implementations. That way run_first_supported can return the value of the implementation that succeeded.
I like that idea 👍
idanarye
force-pushed
the
allow-configuring-the-finder-in-the-plugin-setup
branch
3 times, most recently
from
8983f44
to
71bf489
Compare
| local result = { pcall(impl_function) } | ||
| if result[1] then | ||
| return select(2, unpack(result)) | ||
| elseif result[2] == IMPLEMENTATION_UNAVAILABLE then | ||
| table.insert(unavailable, impl_name) | ||
| else | ||
| error(result[2]) |
There was a problem hiding this comment.
How about this?
| local result = { pcall(impl_function) } | |
| if result[1] then | |
| return select(2, unpack(result)) | |
| elseif result[2] == IMPLEMENTATION_UNAVAILABLE then | |
| table.insert(unavailable, impl_name) | |
| else | |
| error(result[2]) | |
| local supported, result = pcall(impl_function) | |
| if supported then | |
| return result | |
| elseif result == IMPLEMENTATION_UNAVAILABLE then | |
| table.insert(unavailable, impl_name) | |
| else | |
| error(result) |
There was a problem hiding this comment.
But then we won't be able to use multiple returns from the implementation functions. _run_with_finder_backend doesn't use that (it doesn't even return a single value), but run_first_supported is a library function so I tried to make it as generic as possible.
Then again, maybe I'm just prematurely generalizing things...
There was a problem hiding this comment.
Oooh, I see. Ok this is fine with me
|
Looks like there's a styling and linting issue |
idanarye
force-pushed
the
allow-configuring-the-finder-in-the-plugin-setup
branch
from
71bf489
to
e2c04c4
Compare
idanarye
force-pushed
the
allow-configuring-the-finder-in-the-plugin-setup
branch
from
3a3dd51
to
7b07a6c
Compare
|
@epwalsh There is one warning left, but it was not introduced by this PR ( |
|
Ah, sorry about that. Fixed. |
…h#178) Closes epwalsh#175 Closes epwalsh#177 Closes epwalsh#173 Closes epwalsh#169 Closes epwalsh#161 Closes epwalsh#144 Closes epwalsh#138 Fixes epwalsh#136 Closes epwalsh#137 Closes epwalsh#131 Closes epwalsh#117 Closes epwalsh#130 Closes epwalsh#115 Closes epwalsh#86 Closes epwalsh#105 Closes epwalsh#70 Fixes epwalsh#176 Fixes epwalsh#174 Fixes epwalsh#160 Fixes epwalsh#158
No description provided.