attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.44k stars 266 forks source link

nomdl: can't round-trip ref types #3851

Open aboodman opened 5 years ago

aboodman commented 5 years ago
    noms := types.NewValueStore((&chunks.TestStorage{}).NewView())
    emptyBlob := noms.WriteValue(types.NewEmptyBlob(noms))
    foo := types.NewStruct("", types.StructData{"b": emptyBlob})
    s := types.EncodedValue(foo)
    fmt.Println(s) // Prints: struct {b: #2eulo8v8rihcjm0e93brv14dopakkder}
    v := nomdl.MustParse(noms, s)
    fmt.Println(types.EncodedValue(v))

@arv - am I missing something?

arv commented 5 years ago

What happens in the last Println?

aboodman commented 5 years ago

Oh sorry, it crashes, complaining that it doesn't expect # after b:.

aboodman commented 5 years ago

That is, it doesn't make it to the final Println.

arv commented 5 years ago

Both the BNF and parseValue is missing support for Ref:

https://github.com/attic-labs/noms/blob/a1f990c94dcc03f9f1845d25a55e84108f1be673/go/nomdl/parser.go#L263-L271

I don’t remember if this was intentional or just an oversight?

aboodman commented 5 years ago

Does the current human-readable serialization compatible with adding the support? Or would it need to be changed?

On Mon, Jul 29, 2019 at 8:42 AM Erik Arvidsson notifications@github.com wrote:

Both the BNF and parseValue is missing support for Ref:

https://github.com/attic-labs/noms/blob/a1f990c94dcc03f9f1845d25a55e84108f1be673/go/nomdl/parser.go#L263-L271

I don’t remember if this was intentional or just an oversight?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/attic-labs/noms/issues/3851?email_source=notifications&email_token=AAATUBHYJFER4UGOXPQAB53QB422DA5CNFSM4IHK2PPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD3BUMPY#issuecomment-516113983, or mute the thread https://github.com/notifications/unsubscribe-auth/AAATUBESIMMDNMILXSL7Y7LQB422DANCNFSM4IHK2PPA .

arv commented 5 years ago

The current human-readable serialization looks fine and should have no grammar ambiguities.