modify get_line_offset to deal with a non-comment token in a same lin…

[huibing]

If there is a block like this:

/begin CHARACTERISTIC
      /* Name                   */      xxxxxxxxxxxxx
      /* Long Identifier        */      "xxxxxxx"
      /* Type                   */      VALUE 
      /* ECU Address            */      0x0000 /* @ECU_Address@xxxxxxxx@ */ 
      /* Record Layout          */      Scalar_FLOAT32_IEEE 
      /* Maximum Difference     */      0 
      /* Conversion Method      */      single 
      /* Lower Limit            */      -3.4E+38 
      /* Upper Limit            */      3.4E+38
/end CHARACTERISTIC`

when using a2ltool merge it will transform to the following format, in one line:

/begin CHARACTERISTIC xxxxxxxxxxxxx "xxxxxxx" VALUE 0x0000 Scalar_FLOAT32_IEEE 0 single 0 1
    /end CHARACTERISTIC

this pull request will resolve this problem.

[DanielT]

Hi, thanks for the pull request.

I understand what you're trying to do, but I think this change is incorrect.

It will (presumably - no unit test!) work in the situation you described above, but it will break the formatting when comments are preserved between optional blocks.
Example:

/begin CHARACTERISTIC
      xxxxxxxxxxxxx "xxxxxxx" VALUE  0x0000  Scalar_FLOAT32_IEEE  0  single -3.4E+38  3.4E+38
     /* this characteristic is read only for very important reasons - this comment is preserved! */
    READ_ONLY
/end CHARACTERISTIC`

It will also not work if there are two comments in a row for some reason:

/begin CHARACTERISTIC
      /* Name                   */      xxxxxxxxxxxxx
      /* longer description here */
      /* Long Identifier        */      "xxxxxxx"
      /* Type                   */      VALUE 
      /* ECU Address            */      0x0000 /* @ECU_Address@xxxxxxxx@ */ 
      /* Record Layout          */      Scalar_FLOAT32_IEEE 
      /* Maximum Difference     */      0 
      /* Conversion Method      */      single 
      /* Lower Limit            */      -3.4E+38 
      /* Upper Limit            */      3.4E+38
/end CHARACTERISTIC`

A better approach would be to track how many tokens were skipped in Parser::expect_token(), and then look back by that many in get_line_offset.

Additionally, it would be good to have

• a comment describing the handling you're adding in get_line_offset
• a unit test that verifies each of the scenarios described above

[huibing]

Hi, thanks for the pull request.

I understand what you're trying to do, but I think this change is incorrect.

It will (presumably - no unit test!) work in the situation you described above, but it will break the formatting when comments are preserved between optional blocks. Example:

/begin CHARACTERISTIC
      xxxxxxxxxxxxx "xxxxxxx" VALUE  0x0000  Scalar_FLOAT32_IEEE  0  single -3.4E+38  3.4E+38
     /* this characteristic is read only for very important reasons - this comment is preserved! */
    READ_ONLY
/end CHARACTERISTIC`

It will also not work if there are two comments in a row for some reason:

/begin CHARACTERISTIC
      /* Name                   */      xxxxxxxxxxxxx
      /* longer description here */
      /* Long Identifier        */      "xxxxxxx"
      /* Type                   */      VALUE 
      /* ECU Address            */      0x0000 /* @ECU_Address@xxxxxxxx@ */ 
      /* Record Layout          */      Scalar_FLOAT32_IEEE 
      /* Maximum Difference     */      0 
      /* Conversion Method      */      single 
      /* Lower Limit            */      -3.4E+38 
      /* Upper Limit            */      3.4E+38
/end CHARACTERISTIC`

A better approach would be to track how many tokens were skipped in Parser::expect_token(), and then look back by that many in get_line_offset.

Additionally, it would be good to have

• a comment describing the handling you're adding in get_line_offset
• a unit test that verifies each of the scenarios described above

Understood, very valid points here. I will close this for now , and pull again with a better implementation.

[huibing]

Hi,
A new implementation is proposed to handle this kind of problem, plz review if it is ok.
Also,

• Comments are added in the code
• Tests are also added in the parser.rs to verify a few of scenarioes.

[DanielT]

Thanks for the update