Skip to content

Implement Traits to Simplify Code in promptforge-api-types - #63

Open
seanpar203 wants to merge 7 commits into
cppalliance:masterfrom
seanpar203:api-types-traits
Open

seanpar203 wants to merge 7 commits into
cppalliance:masterfrom
seanpar203:api-types-traits

Conversation

@seanpar203

Copy link
Copy Markdown
Contributor

What was done

I implemented several traits for the Timestamp, this allows to simplify the code and do things like Timestamp::now() and it will automatically pull the system epoch time instead of how it was before.

And some similar simplifications to other things like ToolId andCapabilityId.

Before & After TimeStamp

Before

// Manual boilerplate in every caller that records timestamps:
let timestamp = SystemTime::now()
    .duration_since(UNIX_EPOCH)
    .ok()
    .and_then(|elapsed| i64::try_from(elapsed.as_millis()).ok())
    .map_or(Timestamp::UNIX_EPOCH, Timestamp::from_unix_millis);

After

// Direct construction for current time:
let timestamp = Timestamp::now();

// Or convert from an existing SystemTime:
let timestamp = Timestamp::from(some_system_time);

Before & After (ToolId, CapaabilityId, and GlobalName)

Before

// Only inherent methods were available:
let tool_id = ToolId::parse("promptforge/web/fetch")?;
let cap_id = CapabilityId::parse("promptforge/web")?;
let name = GlobalName::parse("promptforge/web/fetch")?;

// This standard Rust idiom DID NOT compile:
// let tool_id: ToolId = "promptforge/web/fetch".parse()?; // ERROR: FromStr not implemented
// let ids: Vec<ToolId> = raw_strings.iter().map(|s| s.parse()).collect::<Result<_, _>>()?; // ERROR

After

// Standard Rust type inference & parsing:
let tool_id: ToolId = "promptforge/web/fetch".parse()?;
let cap_id: CapabilityId = "promptforge/web".parse()?;
let name: GlobalName = "promptforge/web/fetch".parse()?;

// Seamless use with iterators and combinators:
let tool_ids: Result<Vec<ToolId>, _> = ["promptforge/web/fetch", "promptforge/fs/read"]
    .iter()
    .map(|s| s.parse())
    .collect();

This branch has not been deployed

No deployments
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.

1 participant