Skip to content

Commit

Permalink
Fix handling of line comments in bare keys
Browse files Browse the repository at this point in the history 
This change allows the following document to correctly roundtrip:

  foo:
    # Head comment of following map
    - # Head comment of bar
      bar: # Line comment of bar
        baz # Line comment of baz

The logic for indentation was also tuned so it's again more regular.
  • Loading branch information
@niemeyer
niemeyer committed Jun 5, 2020
1 parent e307989 commit a5ece68
Show file tree
Hide file tree
Showing 6 changed files with 120 additions and 29 deletions.
54 changes: 42 additions & 12 deletions emitterc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,10 +235,13 @@ func yaml_emitter_increase_indent(emitter *yaml_emitter_t, flow, indentless bool
emitter.indent = 0
}
} else if !indentless {
emitter.indent += emitter.best_indent
// [Go] If inside a block sequence item, discount the space taken by the indicator.
if emitter.best_indent > 2 && emitter.states[len(emitter.states)-1] == yaml_EMIT_BLOCK_SEQUENCE_ITEM_STATE {
emitter.indent -= 2
// [Go] This was changed so that indentations are more regular.
if emitter.states[len(emitter.states)-1] == yaml_EMIT_BLOCK_SEQUENCE_ITEM_STATE {
// The first indent inside a sequence will just skip the "- " indicator.
emitter.indent += 2
} else {
// Everything else aligns to the chosen indentation.
emitter.indent = emitter.best_indent*((emitter.indent+emitter.best_indent)/emitter.best_indent)
}
}
return true
Expand Down Expand Up @@ -725,16 +728,9 @@ func yaml_emitter_emit_flow_mapping_value(emitter *yaml_emitter_t, event *yaml_e
// Expect a block item node.
func yaml_emitter_emit_block_sequence_item(emitter *yaml_emitter_t, event *yaml_event_t, first bool) bool {
if first {
// [Go] The original logic here would not indent the sequence when
// inside a mapping. In Go we always indent it.
indentless := false
original := emitter.indent
if !yaml_emitter_increase_indent(emitter, false, indentless) {
if !yaml_emitter_increase_indent(emitter, false, false) {
return false
}
if emitter.indent > original+2 {
emitter.indent -= 2
}
}
if event.typ == yaml_SEQUENCE_END_EVENT {
emitter.indent = emitter.indents[len(emitter.indents)-1]
Expand Down Expand Up @@ -785,6 +781,13 @@ func yaml_emitter_emit_block_mapping_key(emitter *yaml_emitter_t, event *yaml_ev
if !yaml_emitter_write_indent(emitter) {
return false
}
if len(emitter.line_comment) > 0 {
// [Go] A line comment was provided for the key. That's unusual as the
// scanner associates line comments with the value. Either way,
// save the line comment and render it appropriately later.
emitter.key_line_comment = emitter.line_comment
emitter.line_comment = nil
}
if yaml_emitter_check_simple_key(emitter) {
emitter.states = append(emitter.states, yaml_EMIT_BLOCK_MAPPING_SIMPLE_VALUE_STATE)
return yaml_emitter_emit_node(emitter, event, false, false, true, true)
Expand All @@ -810,6 +813,29 @@ func yaml_emitter_emit_block_mapping_value(emitter *yaml_emitter_t, event *yaml_
return false
}
}
if len(emitter.key_line_comment) > 0 {
// [Go] A line comment was previously provided for the key. Handle it before
// the value so the inline comments are placed correctly.
if yaml_emitter_silent_nil_event(emitter, event) && len(emitter.line_comment) == 0 {
// Nothing other than the line comment will be written on the line.
emitter.line_comment = emitter.key_line_comment
emitter.key_line_comment = nil
} else {
// An actual value is coming, so emit the comment line.
emitter.line_comment, emitter.key_line_comment = emitter.key_line_comment, emitter.line_comment
if !yaml_emitter_process_line_comment(emitter) {
return false
}
emitter.line_comment, emitter.key_line_comment = emitter.key_line_comment, emitter.line_comment
// Indent in unless it's a block that will reindent anyway.
if event.sequence_style() == yaml_FLOW_SEQUENCE_STYLE || (event.typ != yaml_MAPPING_START_EVENT && event.typ != yaml_SEQUENCE_START_EVENT) {
emitter.indent = emitter.best_indent*((emitter.indent+emitter.best_indent)/emitter.best_indent)
if !yaml_emitter_write_indent(emitter) {
return false
}
}
}
}
emitter.states = append(emitter.states, yaml_EMIT_BLOCK_MAPPING_KEY_STATE)
if !yaml_emitter_emit_node(emitter, event, false, false, true, false) {
return false
Expand All @@ -823,6 +849,10 @@ func yaml_emitter_emit_block_mapping_value(emitter *yaml_emitter_t, event *yaml_
return true
}

func yaml_emitter_silent_nil_event(emitter *yaml_emitter_t, event *yaml_event_t) bool {
return event.typ == yaml_SCALAR_EVENT && event.implicit && !emitter.canonical && len(emitter.scalar_data.value) == 0
}

// Expect a node.
func yaml_emitter_emit_node(emitter *yaml_emitter_t, event *yaml_event_t,
root bool, sequence bool, mapping bool, simple_key bool) bool {
Expand Down
14 changes: 7 additions & 7 deletions encode_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ var marshalTests = []struct {
"v: \"\"\n",
}, {
map[string][]string{"v": []string{"A", "B"}},
"v:\n - A\n - B\n",
"v:\n - A\n - B\n",
}, {
map[string][]string{"v": []string{"A", "B\nC"}},
"v:\n - A\n - |-\n B\n C\n",
"v:\n - A\n - |-\n B\n C\n",
}, {
map[string][]interface{}{"v": []interface{}{"A", 1, map[string][]int{"B": []int{2, 3}}}},
"v:\n - A\n - 1\n - B:\n - 2\n - 3\n",
"v:\n - A\n - 1\n - B:\n - 2\n - 3\n",
}, {
map[string]interface{}{"a": map[interface{}]interface{}{"b": "c"}},
"a:\n b: c\n",
Expand Down Expand Up @@ -164,10 +164,10 @@ var marshalTests = []struct {
"a: 1\n",
}, {
&struct{ A []int }{[]int{1, 2}},
"a:\n - 1\n - 2\n",
"a:\n - 1\n - 2\n",
}, {
&struct{ A [2]int }{[2]int{1, 2}},
"a:\n - 1\n - 2\n",
"a:\n - 1\n - 2\n",
}, {
&struct {
B int "a"
Expand Down Expand Up @@ -420,7 +420,7 @@ var marshalTests = []struct {
// Check indentation of maps inside sequences inside maps.
{
map[string]interface{}{"a": map[string]interface{}{"b": []map[string]int{{"c": 1, "d": 2}}}},
"a:\n b:\n - c: 1\n d: 2\n",
"a:\n b:\n - c: 1\n d: 2\n",
},

// Strings with tabs were disallowed as literals (issue #471).
Expand Down Expand Up @@ -586,7 +586,7 @@ var marshalerTests = []struct {
value interface{}
}{
{"_:\n hi: there\n", map[interface{}]interface{}{"hi": "there"}},
{"_:\n - 1\n - A\n", []interface{}{1, "A"}},
{"_:\n - 1\n - A\n", []interface{}{1, "A"}},
{"_: 10\n", 10},
{"_: null\n", nil},
{"_: BAR!\n", "BAR!"},
Expand Down
62 changes: 56 additions & 6 deletions node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,55 @@ var nodeTests = []struct {
}},
}},
},
}, {
"a:\n # HM\n - # HB1\n # HB2\n b: # IB\n c # IC\n",
yaml.Node{
Kind: yaml.DocumentNode,
Line: 1,
Column: 1,
Content: []*yaml.Node{{
Kind: yaml.MappingNode,
Tag: "!!map",
Line: 1,
Column: 1,
Content: []*yaml.Node{{
Kind: yaml.ScalarNode,
Style: 0x0,
Tag: "!!str",
Value: "a",
Line: 1,
Column: 1,
}, {
Kind: yaml.SequenceNode,
Tag: "!!seq",
Line: 3,
Column: 3,
Content: []*yaml.Node{{
Kind: yaml.MappingNode,
Tag: "!!map",
HeadComment: "# HM",
Line: 5,
Column: 5,
Content: []*yaml.Node{{
Kind: yaml.ScalarNode,
Tag: "!!str",
Value: "b",
HeadComment: "# HB1\n# HB2",
LineComment: "# IB",
Line: 5,
Column: 5,
}, {
Kind: yaml.ScalarNode,
Tag: "!!str",
Value: "c",
LineComment: "# IC",
Line: 6,
Column: 7,
}},
}},
}},
}},
},
}, {
"- a\n- b\n",
yaml.Node{
Expand Down Expand Up @@ -1090,13 +1139,14 @@ var nodeTests = []struct {
Tag: "!!seq",
Line: 6,
Column: 3,
HeadComment: "# HL1\n# HA1",
HeadComment: "# HL1",
Content: []*yaml.Node{{
Kind: yaml.ScalarNode,
Tag: "!!str",
Line: 6,
Column: 5,
Value: "la",
Kind: yaml.ScalarNode,
Tag: "!!str",
Line: 6,
Column: 5,
Value: "la",
HeadComment: "# HA1\n",
}, {
Kind: yaml.ScalarNode,
Tag: "!!str",
Expand Down
12 changes: 8 additions & 4 deletions parserc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -648,6 +648,10 @@ func yaml_parser_parse_node(parser *yaml_parser_t, event *yaml_event_t, block, i
implicit: implicit,
style: yaml_style_t(yaml_BLOCK_MAPPING_STYLE),
}
if parser.stem_comment != nil {
event.head_comment = parser.stem_comment
parser.stem_comment = nil
}
return true
}
if len(anchor) > 0 || len(tag) > 0 {
Expand Down Expand Up @@ -700,9 +704,10 @@ func yaml_parser_parse_block_sequence_entry(parser *yaml_parser_t, event *yaml_e
if token == nil {
return false
}
if prior_head > 0 && token.typ == yaml_BLOCK_SEQUENCE_START_TOKEN {
// [Go] It's a sequence under a sequence entry, so the former head comment
// is for the list itself, not the first list item under it.
if prior_head > 0 && (token.typ == yaml_BLOCK_SEQUENCE_START_TOKEN || token.typ == yaml_BLOCK_MAPPING_START_TOKEN) {
// [Go] It's a sequence or map under a sequence entry, so the former
// head comment is for the underlying sequence or map itself,
// not its first item as the normal logic handles it.
parser.stem_comment = parser.head_comment[:prior_head]
if len(parser.head_comment) == prior_head {
parser.head_comment = nil
Expand All @@ -711,7 +716,6 @@ func yaml_parser_parse_block_sequence_entry(parser *yaml_parser_t, event *yaml_e
// further bytes to the prefix in the stem_comment slice above.
parser.head_comment = append([]byte(nil), parser.head_comment[prior_head+1:]...)
}

}
if token.typ != yaml_BLOCK_ENTRY_TOKEN && token.typ != yaml_BLOCK_END_TOKEN {
parser.states = append(parser.states, yaml_PARSE_BLOCK_SEQUENCE_ENTRY_STATE)
Expand Down
5 changes: 5 additions & 0 deletions scannerc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,11 @@ func yaml_parser_fetch_next_token(parser *yaml_parser_t) (ok bool) {
if !ok {
return
}
if len(parser.tokens) > 0 && parser.tokens[len(parser.tokens)-1].typ == yaml_BLOCK_ENTRY_TOKEN {
// Sequence indicators alone have no line comments. It becomes
// a head comment for whatever follows.
return
}
if !yaml_parser_scan_line_comment(parser, comment_mark) {
ok = false
return
Expand Down
2 changes: 2 additions & 0 deletions yamlh.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -787,6 +787,8 @@ type yaml_emitter_t struct {
foot_comment []byte
tail_comment []byte

key_line_comment []byte

// Dumper stuff

opened bool // If the stream was already opened?
Expand Down

0 comments on commit a5ece68

Please sign in to comment.