Add comments to Enum fields

[maxlandon]

Hello, first thanks a lot for this plugin, really useful at no cost, great stuff !

However in when getting completion and associated documentation strings in an editor, Enum
fields do not have their associated comment. The comment presented is always the one for
the root Enum type name (eg. enum Channel {}).
I would gladly fix this if I could, but I don't quite have the time to get into the codebase, and it
seems to be quite specialized stuff. At the same time I feel that this won't take long to fix (having
written console applications, I might have a sixth sense for this), and this is why I'd like to request
this feature.
I will be immensely grateful for this, as for a project I rely on Enum fields that have important associated
documentation comments, which are absolutely paramount to users that will edit those files.

Thanks a lot for any news !

[chrusty]

Hello @maxlandon, thanks for reporting this. I'll take a look in the coming week. Hopefully this is something fairly simple. There is a bit of a branch in the logic to handle ENUMs, I can easily imagine how this may have happened.

[chrusty]

Hey @maxlandon, are you able to try this branch out and make sure it does what you need? If it looks good then let me know and I'll merge then make a new release.

[chrusty]

Hi @maxlandon. The PR I raised this morning does introduce the comment for an ENUM to the description for its JSONSchema, but it is possible that I'm not entirely understanding what you expect here. Are you able to provide a sample proto, and a sample of what you would like it to turn into?

Here is an example in the unit tests where you can see a code comment for an ENUM proto becoming a JSONSchema description: https://github.com/chrusty/protoc-gen-jsonschema/blob/enum_source_descriptions/internal/converter/testdata/enum_imported.go#L28

[chrusty]

I'm trying their

│       "Type": {
│       │   "oneOf": [
│       │       │   { "const": "Session", "description": "This is a Session description" },
│       │       │   { "const": 0, "description": "This is a Session description" },
│       │       │   { "const": "Beacon", "description": "This is a Beacon description" },
│       │       │   { "const": 1, "description": "This is a Beacon description" }
│       │   ],

I'm just saving the Schema as is, but it doesn't work in my editor. So I don't really know how to test that... I'm not quite familiar with how subschemas work, so part of the thread I linked to is a bit cryptic to me !

After skimming through that trail for the json-schema-spec I think I agree - the const approach seems to be the cleanest option, but for compatibility I'd still have to provide the "ENUM" array too. In any case this probably isn't going to happen any time soon because const doesn't seem to be supported by the jsonschema library I'm using in this plugin.

Interestingly there was an issue raised there a while back along these same lines.

Originally posted by @chrusty in #98 (comment)

[chrusty]

I've merged that other PR, lets bring the ENUM conversation back into this issue from here on.

To summarise, @maxlandon would like to be able to provide an annotation / description for each allowed value in an ENUM, but the JSONSchema spec doesn't allow this (ENUMs are a simple array of values with one overall description field available).

The example above (from a thread on the jsonschema spec repo) alludes to a way to achieve this using const definitions in the "oneOf" for an ENUM, but this isn't supported by our jsonschema library at this stage.

@maxlandon also reports that a manual test of this solution didn't appear to be supported by the editor they were using, which isn't a good sign.

[chrusty]

I've just tried this here - works with draft-05 spec

{
    "$schema": "http://json-schema.org/draft-05/schema#",
    "enum": [
        "VALUE_0",
        0,
        "VALUE_1",
        1,
        "VALUE_2",
        2,
        "VALUE_3",
        3
    ],
    "oneOf": [
        {"const": "VALUE_0", "title": "Zero"},
        {"const": "VALUE_1", "title": "One"},
        {"const": "VALUE_2", "title": "Two"},
        {"const": "VALUE_3", "title": "Three"},
        {"const": 0, "title": "Zero"},
        {"const": 1, "title": "One"},
        {"const": 2, "title": "Two"},
        {"const": 3, "title": "Three"}
    ]
}

[maxlandon]

Ohh thanks a lot for keeping that on top, I appreciate a lot !
Ok let me try this attempt, coming back

[maxlandon]

Okay so I could not fetch the draft-05, (returns 404) so I tried with draft-06.
The same code compiles on the json-schema.org interative compiler, but no results in my editor (vim, if that helps)

EDIT: Actually compiles in the interactive site even with draft-07, and this for both "title" as well as "description" fields.
None of them yield a description in the editor though, so I'm stuck at point 0...

EDIT 2: Does that point to what you said about your dependencies not supporting this ?

[chrusty]

Okay so I could not fetch the draft-05, (returns 404) so I tried with draft-06. The same code compiles on the json-schema.org interative compiler, but no results in my editor (vim, if that helps)

EDIT: Actually compiles in the interactive site even with draft-07, and this for both "title" as well as "description" fields. None of them yield a description in the editor though, so I'm stuck at point 0...

EDIT 2: Does that point to what you said about your dependencies not supporting this ?

Yeah, we're on the right track. It turns out that draft-05 isn't really a thing, so i'm working with Draft-06 here. They maintain decent release notes to work from, and it doesn't look like I'll have any major issues.

I've actually got a PR which will create this schema outlined above, so if that works then we could be in luck.

Which timezone are you in by the way?

[chrusty]

Have a look at this PR: #99

I've introduced a new MessageOption and EnumOption called enums_as_constants. If you take your ENUMs or Messages with one of those then the generated schema will encode the OneOfs as CONST, including the comment/annotation.

I've updated the README for custom proto options too, with links to the sample protos which show how this works.

Hopefully this solves the problem! The generated schemas work with the tooling I use. Let me know once you've had a chance to test this out, then I can think about how to release this without causing too much drama.

[maxlandon]

Hello @chrusty ! Glad to hear this from you ! I'm in France, UTC+1
So good news your PR works... but my Editor completion engine doesn't :)
Nonetheless I can confirm that the descriptions are correctly generated for my proto enums.

... As far as my editor is concerned and me abusing a liiiiiiittttttlle bit of your kindness (your fixes were quick, I appreciate immensely) and ask if you know if the key description works ? It seems the title key is not triggering my editor (it probably only triggers on description key).
Anyway that's my own problems, I'll dig into this...

Thanks a huge lot for your prompt responses !

[chrusty]

Hey @maxlandon, I've changed it from title to description now. Can you try again?

I'm in New Zealand by the way.

[maxlandon]

Hey really cool, far from here but definitely a place to go once. At the very least if the Lord of The Rings is your favorite film :)

For the code, well it doesn't work either in my editor, which means that I'll deal with it, engine I'm using must not be checking the enum values (likely, because until now there wasn't much support for this, as we've seen).
I'll deal with that, I've opened an issue, the "description" vs "title" choice is up to you :) (though probably better to keep description I guess)

Anyway, big thanks for everything ! will help me a lot down the road !

[chrusty]

I've made a new release which includes this new feature.

Thanks for working with me on this one @maxlandon!