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

[ENHANCEMENT] Implement From<T> for UrlParts enums #24

Open
russcam opened this issue Nov 29, 2019 · 3 comments · May be fixed by #126
Open

[ENHANCEMENT] Implement From<T> for UrlParts enums #24

russcam opened this issue Nov 29, 2019 · 3 comments · May be fixed by #126
Labels
discuss General discussion to reach decision enhancement New feature or request

Comments

@russcam
Copy link
Contributor

russcam commented Nov 29, 2019

Each API models the API url parts as an enum, and where an API has more than one enum variant, the API function on the root/namespace client takes the enum as an argument. For example, for search

let response = client
    .search(SearchUrlParts::Index(&["index-1"])) // <-- accepts enum
    .send()
    .await?;

Currently, the Rust compiler cannot infer the enum type based on the parameter, meaning the complete enum path needs to be specified as above, instead of simply the following pseudo

let response = client
    .search(Index(&["index-1"]))
    .send()
    .await?;

To make the API somewhat simpler to use, it is proposed to implement From<T> traits for each enum such that a value, or tuple of values, can be used. Taking the example above

impl<'a> From<&'a [&'a str]> for SearchUrlParts<'a> {
    fn from(index: &'a [&'a str]) -> Self {
        SearchUrlParts::Index(index)
    }
}

impl<'a> From<(&'a [&'a str], &'a [&'a str])> for SearchUrlParts<'a> {
    fn from(index_type: (&'a [&'a str], &'a [&'a str])) -> Self {
        SearchUrlParts::IndexType(index_type.0, index_type.1)
    }
}

let response = client
    .search(&["posts"].into())
    .send()
    .await?;
@russcam russcam added enhancement New feature or request discuss General discussion to reach decision labels Nov 29, 2019
@mwilliammyers
Copy link
Contributor

mwilliammyers commented Jan 9, 2020

I was thinking the exact same thing when I was reading the docs.

Should we go a step further and have search accept an Into<SearchUrlParts<'a>>?

Also, what should we do when the variants are the same type?

@russcam
Copy link
Contributor Author

russcam commented Jan 22, 2020

Should we go a step further and have search accept an Into<SearchUrlParts<'a>>?

👍 to this

Also, what should we do when the variants are the same type?

As far as I am aware, I think there are only a few APIs where the enum variant to construct from a passed tuple of values is ambiguous

  • NodesInfoParts:
    pub enum NodesInfoParts<'b> {
    #[doc = "No parts"]
    None,
    #[doc = "NodeId"]
    NodeId(&'b [&'b str]),
    #[doc = "Metric"]
    Metric(&'b [&'b str]),
    #[doc = "NodeId and Metric"]
    NodeIdMetric(&'b [&'b str], &'b [&'b str]),
    }
  • NodesStatsParts:
    pub enum NodesStatsParts<'b> {
    #[doc = "No parts"]
    None,
    #[doc = "NodeId"]
    NodeId(&'b [&'b str]),
    #[doc = "Metric"]
    Metric(&'b [&'b str]),
    #[doc = "NodeId and Metric"]
    NodeIdMetric(&'b [&'b str], &'b [&'b str]),
    #[doc = "Metric and IndexMetric"]
    MetricIndexMetric(&'b [&'b str], &'b [&'b str]),
    #[doc = "NodeId, Metric and IndexMetric"]
    NodeIdMetricIndexMetric(&'b [&'b str], &'b [&'b str], &'b [&'b str]),
    }
  • NodesUsageParts:
    pub enum NodesUsageParts<'b> {
    #[doc = "No parts"]
    None,
    #[doc = "NodeId"]
    NodeId(&'b [&'b str]),
    #[doc = "Metric"]
    Metric(&'b [&'b str]),
    #[doc = "NodeId and Metric"]
    NodeIdMetric(&'b [&'b str], &'b [&'b str]),
    }

In these cases, I would propose choosing the more common variant?

@russcam
Copy link
Contributor Author

russcam commented Jul 16, 2020

Started looking at this in enhancement/24 branch.

russcam added a commit that referenced this issue Jul 20, 2020
This commit implements From<T> for Url parts enums.

Closes #24
russcam added a commit that referenced this issue Jul 20, 2020
This commit implements From<T> for Url parts enums.

Closes #24
@russcam russcam linked a pull request Jul 20, 2020 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss General discussion to reach decision enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants