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

Handle enum values that contain meaningful non-alphanumeric characters #397

Open
Tracked by #395
augustuswm opened this issue Mar 29, 2023 · 5 comments
Open
Tracked by #395

Comments

@augustuswm
Copy link
Contributor

The GitHub API spec contains the following enum for sorting (truncated for clarity):

{
  "name": "sort",
  "description": "Sorts the results ...",
  "in": "query",
  "required": false,
  "schema": {
    "type": "string",
    "enum": [
      "reactions",
      "reactions-+1",
      "reactions--1",
      "reactions-smile"
    ]
  }
}

The enum generated for this results in duplicate variant names:

pub enum SearchIssuesAndPullRequestsSort {
    #[serde(rename = "reactions")]
    Reactions,
    #[serde(rename = "reactions-+1")]
    Reactions1,
    #[serde(rename = "reactions--1")]
    Reactions1,
    #[serde(rename = "reactions-smile")]
    ReactionsSmile,
}

I think this is fixable by updating the sanitize utility in typify to remove the -1 and +1 branches, and performing that conversion as a .replace("-1", "minus1"), etc in the catch all branch instead.

@ahl
Copy link
Collaborator

ahl commented Mar 29, 2023

This is great. Thanks for filing this.

Abstractly what I've been thinking about for this type of situation (and it relates to oxidecomputer/typify#1) is some sort of collective naming ... thingy... into which we put the raw materials and then it outputs a unique collection. The idea is that the collection would retain a bunch of details and then look at the full set to determine what elements were required for uniqueness.

In this case, we'd want the output to be the set of { Reactions, ReactionsPlus1, ReactionsMinus1, ReactionsSmile }, but if the inputs were, say, reactions, reactions-+1 we might want simply { Reactions, Reactions1 }. This is to say: depending on the other members of the collection we'll need more or less context.

This is also true of type for which we have no great name and instead use the "path" i.e. if there's a type FooType with a field bar, we might have a type called FooTypeBar.

@augustuswm
Copy link
Contributor Author

Got it, that make sense, I should have checked through the typify issues first. Solving that generally, for local scopes like variants, and a global scope for component structs would be very useful.

@ahl
Copy link
Collaborator

ahl commented May 27, 2023

I think what you suggesting is a subtle and great idea: we can prototype the larger naming concept first in local contexts such as variant names.

@MikeIvanichev
Copy link

Hi @ahl, I also encountered this issue when trying to get Avoma's spec to work.
They have enum variants like: start and -start to indicate ascending vs descending start times.

I agree with the idea you outlined in typify #695, I'd happily put in some extra work to get a better output.

In the meantime, is there a way to hack together a workaround you would suggest?

@ahl
Copy link
Collaborator

ahl commented Dec 29, 2024

As I noted here, I've evolved my thinking a bit here. Using this start_at and -start_at example, I don't think we're ever going to algorithmically come up with the labels you want here which are probably something like IncreasingStartAt and DecreasingStartAt. Instead, I'd like to provide a way for users to either patch (i.e. inline) or provide another file (i.e. out-of-band) that describes renaming.

I'm going to be thinking about what this might look like, but would welcome proposals. It'a always helpful to have concrete examples such as this one.

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

No branches or pull requests

3 participants