MaybeJustJames / yaml

Work with YAML in Elm.
BSD 3-Clause "New" or "Revised" License
9 stars 5 forks source link

Empty records or dict with indention #28

Closed RDBModel closed 1 year ago

RDBModel commented 1 year ago

It seems like the library only put empty line for such cases: image It seems that it is better to use inline type of dict or record in such cases, like: image

What do you think?

MaybeJustJames commented 1 year ago

Hi @RDBModel! Thanks for the report. Can you tell me what you're doing exactly? Preferably with minimal example code?

RDBModel commented 1 year ago

Thank you for your quick response. I have a quite big model in Elm code and converting it to the string. There is an encoder for 'system' (that entities are on the pictures):

systemEncoder : System -> Encoder
systemEncoder sys =
    record
        [ ("name", string sys.name)
        , ("description", string sys.description)
        , ("relations", list relationEncoder sys.relations)
        , ("containers", dict identity containerEncoder sys.delivery)
        ]

later I am using toString 2 enc to create a string. So encoder creates just empty lines if there is no itesm in sys.relations or sys.delivery

Here is an example. Logs shows that for empty dict or list only \n is added to the final string: image

MaybeJustJames commented 1 year ago

OK, so this is the behaviour you're observing? https://ellie-app.com/k5JJSqgv2Jda1

And you have something consuming this YAML where you need empty lists to be represented as empty lists?

My feeling is that an empty inline list is indeed more correct. But I need to check because this will be a breaking change.

RDBModel commented 1 year ago

Right, actually I am sending the same string (created by toString function) to Monaco editor and later trying to send it back to Eml. I am using encoder, like

systemDecoder : Decoder System
systemDecoder =
  map4 System
    (field "name" string)
    (field "description" string)
    (field "relations" (list relationDecoder))
    (field "containers" (dict deliveryDecoder))

but it is not parsing system-1 properly if there are empty lines instead of [] or {} (it is basically skipping such record).

MaybeJustJames commented 1 year ago

Ah OK! This sounds like it isn't "round tripping". Unfortunately I don't have time to look at this right now but I absolutely will in the next day or 2.

RDBModel commented 1 year ago

Another case that, perhaps, can be improved https://ellie-app.com/k5NYB5sL7zDa1 image If there is a list of records, each record will be rendered at a new line: image It seems like the following looks better: image

MaybeJustJames commented 1 year ago

Another case that, perhaps, can be improved https://ellie-app.com/k5NYB5sL7zDa1 image If there is a list of records, each record will be rendered at a new line: image It seems like the following looks better: image

I agree. Could you make this a separate bug please?

MaybeJustJames commented 1 year ago

Hi @RDBModel , I've pushed a fix. I would appreciate if you could test it to check it matches your expectations please :)

RDBModel commented 1 year ago

@MaybeJustJames thank you very much for your work. It looks great from my perspective. Unfortunately I have no idea how to test it in my project as I don't know how to use unpublished version of the libraries in Elm (is it even possible?). As I know, Elm core team forbids it in 0.19.x version of Elm.

MaybeJustJames commented 1 year ago

Sorry. I was unclear. The way you would test it would be to checkout this project and either use elm repl om some simple examples, or add a file to the tests/ directory with anything you like that is more complete. You don't need to contribute it of course. Just convince yourself its working as you expect now.

RDBModel commented 1 year ago

yep, it is working as expected (at least, from my perspective). Thank you!

ypyl commented 1 year ago

Just checked this one again and found out that the implementation is missing a space between [] or {} and a keyword, e.g.

name:[]

but it should be

name: []

space after the colon

MaybeJustJames commented 1 year ago

@ypyl and @RDBModel thanks. I've published version 2.1.3 with the encoding changes

RDBModel commented 1 year ago

it looks like the fix is broking encoding in the following case: input yaml:

domain:
  name: My domain
  actors:
    actor-1:
      name: Actor
      relations:
        - uses - system-1
  systems:
    system-1:
      name: System 1

The result is after decoding and decoding its back:

domain:   name: My domain
  actors:     actor-1:
      name: Actor
      relations:         - uses - system-1
  systems:     system-1:
      name: System 1
views: {}

It looks like the reason in this commit. If I change it to prefix = True, it is working as expected