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

Triple Slash comments produce comment offset in generated files. #762

Closed
ehallander9591 opened this issue Apr 18, 2017 · 4 comments
Closed

Comments

@ehallander9591
Copy link

protobuf.js version: 6.7.3

This is similar to an issue I had reported concerning, I think, a blank line before an enum offset the comments.

It would seem that a triple slash comment on the message definition line also causes subsequent triple slash comments to offset as well.

message Location {     /// A geo location on Earth
	float latitude = 1;  			/// Latitude in decimal degrees.  Must be >= -90.0 and <= 90.0
	float longitude = 2; 			/// Longitude in decimal degrees.  Must be >= -180.0 and <= 180.0
	int32 altitude_in_meters = 3; 	/// Altitude in meters above ground
}

produces the following

        /**
         * Constructs a new Location.
         * @exports mpxs.model.Location
         * @constructor
         * @param {mpxs.model.Location$Properties=} [properties] Properties to set
         */
        class Location {

            /**
             * Constructs a new Location.
             * @exports mpxs.model.Location
             * @constructor
             * @param {mpxs.model.Location$Properties=} [properties] Properties to set
             */
            constructor(properties?: mpxs.model.Location$Properties);

            /**
             * A geo location on Earth
             * @type {number}
             */
            public latitude: number;

            /**
             * Latitude in decimal degrees.  Must be >= -90.0 and <= 90.0
             * @type {number}
             */
            public longitude: number;

            /**
             * Longitude in decimal degrees.  Must be >= -180.0 and <= 180.0
             * @type {number}
             */
            public altitudeInMeters: number;

.
.
.
<please paste the stack trace of the error if applicable>
@ehallander9591
Copy link
Author

It's probably fair to note that we will probably switch to having /** */ comments on the messages at some point anyway, but for now we have the simple /// line.

Cheers

@dcodeIO
Copy link
Member

dcodeIO commented Apr 18, 2017

The parser looks at trailing comments only for non-block structures (anything that doesn't use { ... } blocks).

You can see this here: https://github.com/dcodeIO/protobuf.js/blob/master/src/parse.js#L297

In your example, this leads to misinterpretation where comments belong, which certainly needs some improvements in detecting compatible comment types. However, your first comment (message Location { /// A geo location on Earth) still won't work even with a fix for wrong offsets.

@ehallander9591
Copy link
Author

We will change to the block comments which is what we should have used for the message anyway. I thought it might be related, but it only looked related.

Cheers

dcodeIO added a commit that referenced this issue Apr 18, 2017
@dcodeIO
Copy link
Member

dcodeIO commented Apr 18, 2017

This adds a little more knowledge about trailing comments to the parser. It should accept:

/** Comment */
message A { ...
/// Comment
message A { ...
{
  string id = 1; /// Comment
}

plus, it prefers comments on top over trailing comments:

{
  /// Preferred
  string id = 1; /// Discarded
}

and, of course, it shouldn't confuse trailing comments anymore.

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

2 participants