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

Fix Comment position with optional is used. #995

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

AkihiroUeda35
Copy link

If a field is optional and has a leading comment, leading comment is written before "has_field".

message Sample {
  // Comment
  optional int32value = 1;
}
typedef struct _SampleMessage {
    /* Comment*/
    bool has_value;
    int32_t value;
} SampleMessage ;

In this condition, Intellisense comment of VS code is shown on has_value field.
With this change, comment position is on appropriate position as shown below.

typedef struct _SampleMessage {
    bool has_value;
    /* Comment*/
    int32_t value;
} SampleMessage ;

@PetteriAimonen
Copy link
Member

I'm not sure which is better. When reading code directly, it makes more sense to have the comment before has_field.

@AkihiroUeda35
Copy link
Author

Thank you for your comment.
Then, how do you think if generated code becomes as shown below?

typedef struct _SampleMessage {
    /* Comment*/
    int32_t value;
    bool has_value;
} SampleMessage ;

@PetteriAimonen
Copy link
Member

That won't work, has_value has to precede the value for the descriptor offsets to remain in range.

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