Upgrading from pre 1.0

[arkanoid87]

I've upgraded binarylang in my project and I'd like to share some feedback that could improve this wonderful project.

About docs:

• * to export names works for both object name and field names, but it was not clear in docs so I had to experiment
• options get/set in name suffix is capitalized in examples, but lowercase in description, and lowercase is what should be
• if a sub parser returns none, field is nil in parent. This should be stated somewhere.

About code: before 1.0 I was using == operator to check parsed data correctness in my test cases and it was easy as decoded stuff was just a tuple hierarchy. Now with ref types I can still test flat objects easily with

myparsedobj[] == MyParser(field: value)[]

but it won't work with nested parsers as it would compare by ref, so I had to solve with a custom == operator like in

type
    ChildObj = object
        field: int
    RefChildObj = ref ChildObj
    MainObj = object
        field: int
        child: RefChildObj
    RefMainObj = ref MainObj

proc `==`(a, b: RefMainObj | RefChildObj): bool =
    for k, f1, f2 in fieldPairs(a[], b[]):
        if f1 != f2:
            return false
    return true

let
    mymain1 = RefMainObj(field: 1, child: RefChildObj(field: 3))
    mymain2 = RefMainObj(field: 1, child: RefChildObj(field: 3))

echo mymain1 == mymain2

For quite complex parsers the size of the type OR gets quite noisy, it would be easier if all types generated by binarylang were subclass of a common object or something like that. I'm also facing the issue when some subparsers are decoded as nil

[arkanoid87]

I've refined the solution by adding this template to my project

template parserEqualOperator*(T: untyped) =
    proc `==`*(a, b: T): bool =
        for _, f1, f2 in fieldPairs(a[], b[]):
            when (f1 is ref) or (f2 is ref):
                if f1.isNil() and f2.isNil():
                    return true
                elif f1.isNil() or f2.isNil():
                    return false
            if f1 != f2:
                return false
        return true

and applying it below a group of nested parsers (struct) generator like

parserEqualOperator(RefMainObj | RefChildObj)

[sealmove]

Thanks for the issue.

• Nim doesn't care about the casing of identifiers (except first character), so it makes no difference.
• Can you elaborate on subparser returning "none"? What exactly do you mean by "none"? And how is this a problem?
• I am aware of tuples being more handy, but generating objects instead of tuples in needed for recursive parsers to work.

[arkanoid87]

• sure uppercase letters in the middle of the template name doesn't make any difference, was just a quick hint to improve consistency in docs, not really an issue here
• I'm nesting structs parsers and I'm also using this operation to parse according the following bytes (not sure if i will switch to sum structs for this, still thinking about it)

  
  template peekcondget*(parse, parsed, output, prefix: untyped) =
  let save = s.getPosition
  let peek = s.readStr(8*prefix.len)
  s.setPosition(save)
  if peek == prefix:
      parse
      output = parsed

template peekcondput*(encode, encoded, output, prefix: untyped) = if not encoded.isNil: output = encoded encode

If peeked bytes doesn't match the sub-parser result is nil and when it matches a sub-parser type object is returned. Behavior is correct but makes comparison of decoded object not obvious as equality operator `==` doesn't work on values for nested ref types, so a custom `==` operator on bynarylang generated type is used to restore old tuple-based behaviour. Consider this testcase:
```nim
import binarylang, bitstreams
import libparse

struct(dateParser):
    s: _ = "DATE"
    s: year(4)
    s: month(2)
    s: day(2)

struct(timeParser):
    s: _ = "TIME"
    s: hours(2)
    s: minutes(2)
    s: seconds(2)

struct(tempParser):
    s: _ = "TEMP"
    s: temperature(3)

struct(windDirParser):
    s: _ = "WINDDIR"
    s: azimuth(3)

struct(windSpeedParser):
    s: _ = "WINDSPEED"
    s: kmh(3)

struct(mainParser):
    *dateParser {peekcond("DATE")}: date
    *timeParser {peekcond("TIME")}: time
    *tempParser {peekcond("TEMP")}: temp
    *windDirParser {peekcond("WINDDIR")}: winddir
    *windSpeedParser {peekcond("WINDSPEED")}: windspeed

parserEqualOperator(MainParser | WindSpeedParser | WindDirParser | 
                    TempParser | TimeParser | DateParser)  

proc decode(data: string): MainParser = mainParser.get(newStringBitStream(data))
proc encode(data: MainParser): string =
    let encodedStream = newStringBitStream()
    mainParser.put(encodedStream, data)
    encodedStream.seek(0)
    encodedStream.readAll

proc decodeRencodeTest(data, name: string) =
    let decoded = data.decode
    assert decoded == decoded, name & " decode comparison"
    let reencoded = decoded.encode
    assert data == reencoded, name & " reencoded comparison"

"".decodeRencodeTest "Empty"
"DATE20210314TIME135056TEMP055WINDDIR071WINDSPEED122".decodeRencodeTest "All Options"
"DATE20210314TIME135056WINDDIR071WINDSPEED122".decodeRencodeTest "Missing one option"
"DATE20210314".decodeRencodeTest "Missing all but head"
"WINDSPEED122".decodeRencodeTest "Missing all but tail"
"TIME135056WINDDIR071".decodeRencodeTest "Missing all but center"
"DATE20210314TEMP055WINDSPEED122".decodeRencodeTest "Missing some"

I'm quite happy with the ref object based approach and my code looks much better without typegetter, what I am trying to say it that I think that comparison of decoded object generated by binarylang should could be made working out-of-the-box/easier without my cumberstone template approach.

[sealmove]

I am closing this issue since this is being discussed at #13.