Comment marshaling and unmarshaling support

[autrilla]

This introduces support for marshaling and unmarshaling comments.

Some key points:

• Comment unmarshaling is opt-in.
• We don't retain indentation information for comments. Comments are placed at the same indentation level as the previous token.
• The implementation uses the exact same structure as all current YAML tokens (so it's tokenized, an event is pushed, etc).

[jvehent]

these import paths changes need to be reverted

[jvehent]

these import path changes need to be reverted

[jvehent]

Tests are failing due to e09b2a6, therefore this is blocked on merging #223

[autrilla]

The tests now pass, cc @jvehent

[buchanae]

I'm missing a comment from the "path" key in the test program below. Notice the comment do show up for all other keys (e.g. "server")

package main

import "fmt"
import "github.com/mozilla-services/yaml"

var data = `
# Database
database:
  # Database path
  path: 'path/to/db'
  # Server port
  server: 9090
  # Sub comment
  sub:
    val: foo
`

func main() {
  b := []byte(data)
  unmarshaler := yaml.CommentUnmarshaler{}
  doc := yaml.MapSlice{}
  err := unmarshaler.Unmarshal(b, &doc)
  if err != nil {
    panic(err)
  }

  ob, _ := yaml.Marshal(doc)
  fmt.Println(doc)
  fmt.Println(string(ob))

  if string(ob) != data {
    panic("Failed assertion")
  }
}

and the output

[{{ Database path} <nil>} {database [{path path/to/db} {{ Server port} <nil>} {server 9090} {{ Sub comment} <nil>} {sub [{val foo} {{ Slice comment} <nil>}]} {sl [one { Two} two]}]}]
# Database path
database:
  path: path/to/db
  # Server port
  server: 9090
  # Sub comment
  sub:
    val: foo
    # Slice comment
  sl:
  - one
  # Two
  - two

panic: Failed assertion

goroutine 1 [running]:
main.main()
	/Users/buchanae/go/src/scratch/potato/potato.go:37 +0x387
exit status 2

Thanks for doing this. I'm hoping to use this to better manage YAML config files. Let me know I can help move this along somehow.

[buchanae]

Oh, I actually just noticed that the "Database path" comment is actually bumped up a level, incorrectly, to the "database" key.

Perhaps this is related to the original comment in this PR:

We don't retain indentation information for comments. Comments are placed at the same indentation level as the previous token.

[autrilla]

Yes, that's what's happening here. Keeping the comment where it was initially place is a harder problem, and it's specially harder considering iirc currently the library doesn't keep tokens on the same "style" they were in. So a round trip might change indentation, or even string style (not particularly sure about this last one).

…

On Tue, Apr 25, 2017 at 7:25 PM Alex Buchanan ***@***.***> wrote: Oh, I actually just noticed that the "Database path" comment is actually bumped up a level, incorrectly, to the "database" key. Perhaps this is related to the original comment in this PR: We don't retain indentation information for comments. Comments are placed at the same indentation level as the previous token. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#219 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACJ-V-IM2mNtcXsUE0clFEWgUkeiYXbDks5rziyLgaJpZM4Knlz9> .

[josephholsten]

@autrilla any progress with this? I'm really keen on replacing my usage of https://pypi.python.org/pypi/yamlfmt/0.1.5 with this

[autrilla]

@josephholsten this is probably never going to get to the level of ruaml.yaml's round trip parser. go-yaml doesn't keep style information, and changing it so it does would be hard. Using this to reformat your yaml will remove some information. I'm not sure if this answers your question, but the progress you see here is all the progress we've made.

[josephholsten]

ah, that's good to know. how would you feel about a fork that did keep that info? Would it make sense for that to live in a separate package, or do you think that might be worth being part of go-yaml?

[autrilla]

Well, this PR has been open for a while now and hasn't received any comments from contributors, so I don't know if there's any plan to ever maintain presentation layer information such as comments. A fork that keeps this info would be great, and you might consider starting from what I worked on. A PR back to mozilla-services' fork that retains more presentation information would be amazing 😄

[lox]

There have been a few discussions on comment handling at #132 and #129.

@niemeyer would the above change be something that might make sense in v3?

[niemeyer]

Yes, this is certainly worth looking into after v3 is out.

Preliminary issues are lack of tests and conflicts.

[Naatan]

What is the timeline on v3? Very interested in this PR.

[khimaros]

I would love to see this PR merged.

[niemeyer]

Thank you very much for the PR and for the discussion. It's clear that everybody wants comment decoding and encoding, and historically it's also clear that there's significant interest in preserving some of the document structure to enable mechanical modifications.

As such, I've been integrating comment decoding/encoding into v3, using the intermediate state management that comes with it. The approach used internally is also slightly different from the one here, in the sense comments are not first-class tokens, but rather metadata associated with existing tokens. This makes it much easier to understand, manipulate, and preserve file comments as desired.

Here is a sneak peek of what's coming:

https://twitter.com/gniemeyer/status/1073638853225996288

I still need to handle a few more edge cases, but given the amount of testing so far I'm comfortable that this is going to work.

On that basis, I'm tentatively closing this PR for the time being. Thanks again for the collaboration here.

[zzak]

@niemeyer Thank you for the update! I'm very excited to see progress here.

Is there anywhere we can try out an early build of v3?

[vdods]

Hi, I'm interested in getting comments in go-yaml as well. Any word on v3?