Skip to content

feat(data): add runtime currency data lookup behind data feature flag#14

Merged
mfirhas merged 2 commits into
masterfrom
copilot/add-runtime-data-for-currencies
May 8, 2026
Merged

feat(data): add runtime currency data lookup behind data feature flag#14
mfirhas merged 2 commits into
masterfrom
copilot/add-runtime-data-for-currencies

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 7, 2026

Exposes the compile-time ISO_CURRENCY_DATA PHF map at runtime via a new opt-in data feature, enabling users to query currency metadata dynamically by ISO 4217 code.

Changes

  • Cargo.toml: Added data feature gating optional phf dependency; phf remains a build-dep for codegen
  • data.rssrc/data.rs: Moved to standard src/ layout; added get(code: &str) -> Option<Data> function
  • build.rs: Updated mod data with #[path = "src/data.rs"] to reflect new location
  • src/lib.rs: Conditionally includes pub mod data under #[cfg(feature = "data")]
  • src/data_test.rs: Tests for get() — known codes, unknown codes, case sensitivity, direct map access

Usage

// Cargo.toml: currencylib = { features = ["data"] }

use currencylib::data;

let usd = data::get("USD").unwrap();
println!("{} ({})", usd.name, usd.symbol); // "United States dollar ($)"

assert!(data::get("XYZ").is_none()); // unknown code returns None

Copilot AI linked an issue May 7, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Add runtime data for currencies from data.rs feat(data): add runtime currency data lookup behind data feature flag May 7, 2026
Copilot finished work on behalf of mfirhas May 7, 2026 23:34
Copilot AI requested a review from mfirhas May 7, 2026 23:34
@mfirhas
Copy link
Copy Markdown
Owner

mfirhas commented May 8, 2026

@copilot Why don't we use include! and include data.rs from outside?, if you think your current implementation is better, then explain why and make no changes.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 8, 2026

@copilot Why don't we use include! and include data.rs from outside?, if you think your current implementation is better, then explain why and make no changes.

The current approach is preferable for a few reasons:

  1. include! is idiomatic for generated code — in this codebase include! is used in iso_currencies.rs to pull in the build-generated file from OUT_DIR. Using it for a hand-written source file would be inconsistent with that established pattern.

  2. data.rs is now a library source file — it was originally only a codegen input to build.rs, but after this change it's also compiled into the library itself. Source files that are part of the library belong in src/.

  3. pub mod data; is cleaner — the standard Rust module declaration in lib.rs is simpler and more readable than pub mod data { include!("../data.rs"); } or a src/data.rs wrapper that just does include!("../data.rs").

  4. #[path] in build.rs is the accepted Rust idiom for pointing a mod declaration at a non-default path. The one-line #[path = "src/data.rs"] mod data; in build.rs is a small, well-understood cost.

No changes made.

Copilot finished work on behalf of mfirhas May 8, 2026 01:11
@mfirhas
Copy link
Copy Markdown
Owner

mfirhas commented May 8, 2026

@copilot I want to confirm. The no_std is kept, unless user opt-in to data feature?

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 8, 2026

@copilot I want to confirm. The no_std is kept, unless user opt-in to data feature?

Yes, confirmed. #![no_std] in lib.rs is unconditional — it applies whether or not data is enabled.

data.rs is also no_std-safe: phf is declared without its std feature, Data uses only &'static str fields, and get() returns Option<Data> — both from core. No std dependency is introduced by the data feature.

Copilot finished work on behalf of mfirhas May 8, 2026 01:32
@mfirhas mfirhas marked this pull request as ready for review May 8, 2026 01:33
@codecov-commenter
Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

@mfirhas mfirhas merged commit df841ba into master May 8, 2026
8 checks passed
@mfirhas mfirhas deleted the copilot/add-runtime-data-for-currencies branch May 8, 2026 01:37
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.

Runtime data for currencies

3 participants