multi_try icon indicating copy to clipboard operation
multi_try copied to clipboard

Optional manual implementation as a feature flag

Open Nerglej opened this issue 1 year ago • 1 comments

Hello!

I really, really like this library, so I made a few tweaks that I needed for a project, even though this project is pretty old by now.

I made a couple of changes:

  • A recursive macro that implements all needed for us, with one call instead of the 25 we made earlier.
  • Wrapped the MultiTry trait itself in a macro, to allow for manually implementing any number of items you need, behind a feature flag to not break api.

When enabling the manual feature, a simple call to the macro is removed from the library. It then needs to be called, as explained in "Manual implementation" in the docs. Since it's a feature-flag, no tests are broken, except for the doc-tests. I decided to ignore this doc-test instead, because it's simple. If anyone has an idea to make it compile, you're very welcome!

I also added a bit of documentation to the other macros that got exported, to warn users, because of limits with rusts macros, when being exported to other crates.

Hope this PR isn't for no use! Have a nice day😊

Nerglej avatar Aug 27 '24 23:08 Nerglej

Thank you for the contribution! If I'm understanding the change properly, this implements:

  • A manual feature so that users can impl the MultiTry trait themselves
    • Presumably to save code space and/or compile time?
  • A recursive macro implementation which implements all "smaller" trait implementations automatically
    • This would potentially be at the expense of code space and/or compile time

Are these two features in conflict? It seems like a user might enable the manual feature to keep the code size minimal, but then be forced to use the recursive macro implementation which would increase their code size.

As for the manual feature, I think other crates solve this by exposing features like multi-try-2, multi-try-3, .., multi-try-26. That was we don't have to expose the implementation of the MultiTry trait to the user, even if it's abstracted by a macro.

Since it's a feature-flag, no tests are broken, except for the doc-tests. I decided to ignore this doc-test instead, because it's simple. If anyone has an idea to make it compile, you're very welcome!

I think this is breaking the doc tests because the doc test has multiple calls to impl_multi_try, which is no longer allowed as of this change (since impl_multi_try does a recursive implementation now by default). Perhaps this is more reason to remove the recursive implementation?

JoshMcguigan avatar Sep 01 '24 17:09 JoshMcguigan