Skip to content

docs: add docs for more mbtiles functions#1849

Merged
CommanderStorm merged 4 commits intomaplibre:mainfrom
CommanderStorm:mbtiles-docs2
May 25, 2025
Merged

docs: add docs for more mbtiles functions#1849
CommanderStorm merged 4 commits intomaplibre:mainfrom
CommanderStorm:mbtiles-docs2

Conversation

@CommanderStorm
Copy link
Member

this PR documents more of the mbtiles crate motivated by @Murtaught

@CommanderStorm CommanderStorm requested review from Copilot and nyurik and removed request for Copilot May 24, 2025 21:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds documentation for additional functions in the mbtiles crate to clarify usage and behaviors for creating and opening mbtiles databases.

  • New documentation for the constructor (new) and file-opening methods (open, open_or_new, open_readonly)
  • Updated comments for stream methods and tile SQL retrieval to improve clarity
  • Added an example for the batch tile insertion method
Comments suppressed due to low confidence (1)

mbtiles/src/mbtiles.rs:328

  • Confirm that the behavior described in the get_tile_and_hash_sql documentation has been updated intentionally. If the implementation now returns null instead of an MD5 hash, consider adding a brief note explaining the rationale for this change.
/// For [`MbtType::Flat`] accessing it is not possible, we thus return null.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enriches the mbtiles crate documentation by adding and updating doc comments for several core methods and accessors, clarifying usage patterns and edge cases.

  • Added docs for constructors and open methods (new, open, open_or_new, open_readonly)
  • Documented accessors (filepath, filename) and operational methods (stream_coords, stream_tiles, insert_tiles)
  • Updated internal method doc for SQL behavior (get_tile_and_hash_sql)

///
/// # async fn foo() {
/// let mbtiles = Mbtiles::new("example.mbtiles").unwrap();
/// let conn = mbtiles.open_or_new().await.unwrap();
Copy link

Copilot AI May 24, 2025

Choose a reason for hiding this comment

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

The example for open_readonly is calling open_or_new. It should call open_readonly() to match the documented method.

Suggested change
/// let conn = mbtiles.open_or_new().await.unwrap();
/// let conn = mbtiles.open_readonly().await.unwrap();

Copilot uses AI. Check for mistakes.
Comment on lines +190 to +196
/// <div class="warning">
///
/// **Note:** The returned [`Stream`] holds a mutable reference to the given
/// connection, making it unusable for anything else until the stream
/// is dropped.
///
/// </div>
Copy link

Copilot AI May 24, 2025

Choose a reason for hiding this comment

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

[nitpick] Raw HTML in Rustdoc can be fragile; consider using Rustdoc admonition syntax (e.g., # Warning or > **Note:**) for better readability and tooling support.

Suggested change
/// <div class="warning">
///
/// **Note:** The returned [`Stream`] holds a mutable reference to the given
/// connection, making it unusable for anything else until the stream
/// is dropped.
///
/// </div>
/// # Warning
///
/// **Note:** The returned [`Stream`] holds a mutable reference to the given
/// connection, making it unusable for anything else until the stream
/// is dropped.
///

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copy link
Member

@nyurik nyurik left a comment

Choose a reason for hiding this comment

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

thanks!! Great docs and great examples. In the future, we may rethink the approach to the mbtiles api - i don't think it is the most optimal at this point:

  • we may want to introduce a cleaner connection state struct, i.e. MbtilesConnection, which will internally know the detected type of the file - so it won't need to be passed around
  • we may want to modify insert_tiles to take &[u8] instead of Vec<u8>

and maybe other refactorings. Current api was tailored to Martin's internal usage, and was never really targeting broader usecases.

@CommanderStorm CommanderStorm merged commit ff6959f into maplibre:main May 25, 2025
20 checks passed
@CommanderStorm CommanderStorm deleted the mbtiles-docs2 branch July 10, 2025 17:03
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.

3 participants