search

attic-labs / noms

The versioned, forkable, syncable database
Apache License 2.0
7.45k stars 267 forks source link

nomdl.Parse(): Incorrect result #3868

Closed aboodman closed 4 years ago

aboodman commented 4 years ago

nomdl.Parse("[map {}]") should return types.NewList(vs, types.NewMap(vs))), but it actually returns an empty list.

aboodman commented 4 years ago

@arv fyi, if you're bored :).

arv commented 4 years ago

I'll try to get my golang env working again

aboodman commented 4 years ago

It should be super easy. I've converted Noms to use Go modules, so it should be easier to get building and so-on than before. You just need to upgrade to a recent Go.

aboodman commented 4 years ago

I just created https://github.com/attic-labs/noms/blob/master/HACKING.md. Untested, but should be 75% correct.

arv commented 4 years ago

Turns out that this fails because of:

https://github.com/attic-labs/noms/pull/3586#discussion_r127086685

The map is empty so it is not added to the list :'(

I wonder what breaks if we change this behavior?

aboodman commented 4 years ago

Bummer. Off the top of my head, the opposite default behavior makes more sense. If you want to remove the empty item container you can just remove it too. Want to try changing it and see if any tests break? If they don't I'm fine risking it.

On Mon, Dec 2, 2019 at 7:54 PM Erik Arvidsson notifications@github.com wrote:

Turns out that this fails because of:

3586 (comment)

https://github.com/attic-labs/noms/pull/3586#discussion_r127086685

The map is empty so it is not added to the list :'(

I wonder what breaks if we change this behavior?

— 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/3868?email_source=notifications&email_token=AAATUBBCV7MKVZ42332YKL3QWXYABA5CNFSM4JQ6JLLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFYFTKY#issuecomment-561011115, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATUBBUDNIME54BPMKQPKLQWXYABANCNFSM4JQ6JLLA .

aboodman commented 4 years ago

Thank-you very much for looking at this, btw. Totally fine if you've had your yearly fill of Noms :). I can take it from here too.