Skip to content

Commit

Permalink
Revert v2 line length change as discussed in #670
Browse files Browse the repository at this point in the history
It was clearly a mistake to accept the default formatting change in v2,
and now there's no ideal choice. Either we revert the change and break
some projects twice, or we keep the change and break some projects once.
Given the report comes from Kubernetes, which has a relevant community
and code size, we'll revert it. At the same time, to simplify the life
of those that already started migrating towards the v3 behavior, a new
FutureLineWrap function is being introduced to trivially preserve the
new behavior where desired.

The v3 branch is not affected by this, and will retain the default
non-wrapping behavior. It will also be changed soon to support per
arbitrary line-wrapping for individual encoding operations.

Thanks to everyone who offered code and ideas, and apologies for
the trouble.
  • Loading branch information
niemeyer committed Nov 17, 2020
1 parent b893565 commit 7649d45
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 10 deletions.
6 changes: 5 additions & 1 deletion apic.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,14 +79,18 @@ func yaml_parser_set_encoding(parser *yaml_parser_t, encoding yaml_encoding_t) {
parser.encoding = encoding
}

var disableLineWrapping = false

// Create a new emitter object.
func yaml_emitter_initialize(emitter *yaml_emitter_t) {
*emitter = yaml_emitter_t{
buffer: make([]byte, output_buffer_size),
raw_buffer: make([]byte, 0, output_raw_buffer_size),
states: make([]yaml_emitter_state_t, 0, initial_stack_size),
events: make([]yaml_event_t, 0, initial_queue_size),
best_width: -1,
}
if disableLineWrapping {
emitter.best_width = -1
}
}

Expand Down
26 changes: 21 additions & 5 deletions encode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -397,11 +397,27 @@ var marshalTests = []struct {
map[string]interface{}{"a": jsonNumberT("bogus")},
"a: bogus\n",
},
// Ensure that strings do not wrap
{
map[string]string{"a": "abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 "},
"a: 'abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 '\n",
},
}

func (s *S) TestLineWrapping(c *C) {
var v = map[string]string{
"a": "abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 ",
}
data, err := yaml.Marshal(v)
c.Assert(err, IsNil)
c.Assert(string(data), Equals,
"a: 'abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 abcdefghijklmnopqrstuvwxyz\n" +
" ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 '\n")

// The API does not allow this process to be reversed as it's intended
// for migration only. v3 drops this method and instead offers more
// control on a per encoding basis.
yaml.FutureLineWrap()

data, err = yaml.Marshal(v)
c.Assert(err, IsNil)
c.Assert(string(data), Equals,
"a: 'abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ 1234567890 '\n")
}

func (s *S) TestMarshal(c *C) {
Expand Down
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
module "gopkg.in/yaml.v2"
module gopkg.in/yaml.v2

require (
"gopkg.in/check.v1" v0.0.0-20161208181325-20d25e280405
)
go 1.15

require gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405
12 changes: 12 additions & 0 deletions yaml.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,3 +464,15 @@ func isZero(v reflect.Value) bool {
}
return false
}

// FutureLineWrap globally disables line wrapping when encoding long strings.
// This is a temporary and thus deprecated method introduced to faciliate
// migration towards v3, which offers more control of line lengths on
// individual encodings, and has a default matching the behavior introduced
// by this function.
//
// The default formatting of v2 was erroneously changed in v2.3.0 and reverted
// in v2.4.0, at which point this function was introduced to help migration.
func FutureLineWrap() {
disableLineWrapping = true
}

0 comments on commit 7649d45

Please sign in to comment.