Skip to content

Conversation

veeshi
Copy link
Contributor

@veeshi veeshi commented Jul 1, 2025

Currently the trait cannot be implemented for object keys that aren't actually a String, (we use Box) this way anything that implements AsRef can implement this trait

@besok besok requested a review from Copilot July 2, 2025 20:29
Copy link

@Copilot 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 updates the Queryable trait and its Value implementation so that as_object returns &str keys rather than &String, allowing any AsRef<str>-backed type (like Box<str>) to implement the trait.

  • Changed Queryable::as_object signature from &String to &str
  • Updated the Queryable for Value impl to call k.as_str() when building the vector
  • Broadens implementability to types beyond String-keyed maps
Comments suppressed due to low confidence (2)

src/query/queryable.rs:73

  • [nitpick] The trait method signature has changed to return &str keys—consider updating any associated doc comments to reflect this new behavior and note that implementers now accept any AsRef<str>.
    fn as_object(&self) -> Option<Vec<(&str, &Self)>>;

src/query/queryable.rs:73

  • [nitpick] We should add unit tests for types with non-String keys (e.g., Box<str>) to verify that as_object correctly returns &str references in practice.
    fn as_object(&self) -> Option<Vec<(&str, &Self)>>;

@besok besok merged commit 346986f into besok:main Jul 2, 2025
5 checks passed
@besok
Copy link
Owner

besok commented Jul 2, 2025

Good catch. Thanks for the pr

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.

2 participants