Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide support for specifying fields in output #376

Open
philippeitis opened this issue Oct 2, 2022 · 3 comments
Open

Provide support for specifying fields in output #376

philippeitis opened this issue Oct 2, 2022 · 3 comments

Comments

@philippeitis
Copy link
Collaborator

philippeitis commented Oct 2, 2022

It would be nice if the APIs provided an include_field API with definitions for fields that could be included. Given that all possible fields are known, it seems like it'd be possible to generate a field enum for each method, which includes all of the possible fields.

This would be helpful for both discoverability (since this might not be the most well-known feature), and correctness (by only including fields which exist as enum variants, every include_field call would be correct, and we can guarantee that the fields parameter syntax would be correct).

Example usage (current):

drive
        .files()
        .list()
        .param("fields", "nextPageToken,files(id,hasThumbnail)")
        .page_size(5);

Example usage (desired):

drive
        .files()
        .list()
        .include_field(ListField::NextPageToken)
        .include_field(ListField::Files(vec![FileField::Id, FileField::HasThumbnail]))
        .page_size(5);
@Byron
Copy link
Owner

Byron commented Oct 4, 2022

That would certainly be nice to have! And it would even be backwards compatible.

The implementation would probably have to use the schema information to build types to build a sibling structure like proposed here. I wonder if it could be made available through the returned structure itself.

For instance, if files().list() returns a File type, could File::FIELD::NextPagetoken be a way to access it? That we we don't have even more types to deal with, hopefully eliminating the potential for clashes entirely.

@philippeitis
Copy link
Collaborator Author

The only obvious solution I can see that would allow for the proposed solution is using associated trait types:

trait Fields {
    type Fields;
}

struct File {
    id: String,
    name: String,
}


trait ToParam {
    fn to_param(&self) -> String;
}

mod fields {
    use super::ToParam;

    pub enum File {
        Id,
        Name
    }

    
    impl ToParam for File {
        fn to_param(&self) -> String {
            match self {
                File::Id => "id",
                File::Name => "name"
            }.to_string()
        }
    }
}


impl Fields for File {
    type Fields = fields::File;
}

fn main() {
    println!("{}", <File as Fields>::Fields::Id.to_param());
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=14d7b70af2bdc88ffdc303c81e01d3b3

This solution could use a separate module for the fields to avoid collisions wherever possible (the names can be further disambiguated by using the full method chain).

Your proposal of using File::FIELD::NextPageToken syntax, which would quite nice (example: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ae62cc52907647b81a09d202ba7eaf4b) is not currently supported, per rust-lang/rust#8995.

However, using the trait-based associated type approach would not prevent File::FIELD::NextPageToken syntax in the future.

@Byron
Copy link
Owner

Byron commented Oct 7, 2022

This looks good to me, and I think it would help tremendously to know exactly how to access a types fields, instead of associating them by naming scheme for example. The latter would of course work, too, i.e. struct File has its field in fields::File or something like that. Maybe both can be employed at the same time as well, leaving using the associated type optional, even for us - after all it ads complexity which might better be avoided or added later once it's clear it's worth the cost.

In any case, my suggestion is to go ahead with a minimal implementation that associates field enums by name, and optionally associate the struct and enums by trait in a follow-up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants