projectfluent / fluent-rs

Rust implementation of Project Fluent
https://projectfluent.org
Apache License 2.0
1.05k stars 95 forks source link

Missing documentation for FluentBundle #169

Closed sbrl closed 3 years ago

sbrl commented 4 years ago

The FluentBundle struct, although present in the introduction text of the documentation, is not actually documented. The link there to FluentBundle sends me here, but I see a 404:

image

I assume that this is a bug?

zbraniecki commented 4 years ago

yep! thank you for reporting it!

XAMPPRocky commented 4 years ago

The issue is that FluentBundle was renamed FluentBundleBase and is no longer publicly accessible in the documentation despite still being available in the public API through the type FluentBundle aliases. FluentBundleBase needs to be publicly re-exported.

zbraniecki commented 4 years ago

I'm not sure how to re-export it so that in the documentation it shows as FluentBundle. The only reason we have the Base is so that we can avoid duplication between FluentBundle and concurrent::FluentBundle, and I'd like to have docs on both be docs on those two names.

Any idea how to approach it?

XAMPPRocky commented 4 years ago

Well as a user I would just prefer FluentBundleBase to be publicly accessible and for that reason you mentioned to be documented in the API with a introduction that redirects people to the type aliases for actual use. It keeps it as single method in the code and in the search and is easy to implement, exposing implementation details aren't great but often the simplest solution. That being said there are options.

For a struct like FluentBundle the methods are the most important thing to be visible for me (I found this issue while trying to find documentation for a method), so you could write and implement a common trait and have that trait publicly accessible but sealed so it can't be implemented.

There's also always the option to implementing it as a macro to generate two distinct concrete types, but I don't think that would be worth the overhead/complexity.

mainrs commented 3 years ago

Any idea how to approach it?

Maybe instead of using a type you should get away by declaring it as so in the lib.rs file:

pub use FluentBundleBase as FluentBundle;

This will definitely work when importing the module. I do not know if you can use this in the same module as the one that defines FluentBundleBase.

zbraniecki commented 3 years ago

I fixed doc links for 0.13, but there's still more to be done to clean up the docs between FluentBundleBase, FluentBundle and concurrent::FluentBundle.

zbraniecki commented 3 years ago

A lot of work went into improving the docs!

See https://docs.rs/fluent-bundle/0.15.0/fluent_bundle/ and https://docs.rs/fluent-syntax/0.11.0/fluent_syntax/

Hope that solves it! Open a new issue/PR when needed