Fix for #2677 by W-Floyd · Pull Request #2705 · mikefarah/yq

W-Floyd · 2026-05-12T22:00:40Z

ccoVeille · 2026-05-13T13:11:12Z

+	if stringValue == "<<" {
+		node.Tag = "!!merge"
+	} else {
+		node.Tag = "!!str"
+	}
 	return node


Could you add tests for this?

Because it's not really obvious

Added a test that replicates what's documented in #2677, will look at doing a more targeted test that exercises just this functionality.

Depends on yaml/go-yaml#348

W-Floyd · 2026-05-13T17:34:19Z

Changed the existing tests to pass given the changes in yaml/go-yaml#348, added a test for the given fix, and found a corner case to improve specific to the example with multiplication. I am still very new to both the go-yaml and yq codebases, and neither am I intimately familiar with the YAML spec, so sorry for the back and forth, and I appreciate the feedback!

It seems when yq autocreates a << key it was creating it with tag=!!str, which go-yaml was handling for us before. Given the changes in http://github.com/yaml/go-yaml/commit/fbd745be58df058ec8eb7d7b42468458f8d8ec64, we need to give it tag=!!merge ourselves. Note that this is only for the nodes yq creates itself, hence why nested alias/anchor stuff (that were handled by go-yaml wholesale) kept working.

Regarding mergeObjects in operator_multiply.go, during a merge operation (*+), the code iterated over all nodes from the RHS and skipped any with tag=!!merge. It seems the intent was to avoid accidentally following merge anchors, but since DontFollowAlias: true is already set on the traversal, that protection was redundant. The problem was that skipping the << key node meant yq never ran the key assignment for it — so instead of copying the source key's style, it autocreated a fresh one with Style=0, silently dropping the explicit !!merge annotation. The fix narrows the skip to !candidate.IsMapKey: values tagged !!merge (hypothetical) are still skipped, but << key nodes are processed normally, and UpdateFrom copies both the tag and the style from the source.

ccoVeille · 2026-05-13T18:19:31Z

@mikefarah @ingydotnet what do you think about what @W-Floyd explained ?

Thanks

I think his analysis is good, but it leads me to question the change we did in go-yaml with

yaml/go-yaml@fbd745b

It might be OK, and then imply a change in yq like the one suggested here, but I prefer everyone to review the changes here and the one we did in go-yaml

ccoVeille · 2026-05-13T18:23:53Z

I'm very surprised by the commit

eb7738f

That remove the merge and float tags in a lot of files.

It doesn't sound OK.

Maybe it's due to the fact the go-yaml PR needs to be merged first, but seeing such commit that changes so much things make me feel there is something wrong

W-Floyd · 2026-05-13T20:46:13Z

Yes, it is for the sake of yaml/go-yaml#348 - I can back out those changes if they should wait. But neither will the added test work until that's merged.

mikefarah · 2026-05-14T09:17:05Z

 		expression:     `.a = .a % .b`,
 		expected: []string{
-			"D0, P[], (!!map)::{a: !!float 2, b: 2.5}\n",
+			"D0, P[], (!!map)::{a: 2, b: 2.5}\n",


This seems to have change a whole bunch of unrelated things...

Sorry, that's to match the new output of yaml/go-yaml#348 - I can revert it for now if we want to handle that separately.

This sounds strange to me then.

I have commented the go-yaml PR

yaml/go-yaml#348 (comment)

mikefarah · 2026-05-14T09:18:57Z

+		description: "explode expands << merge keys regardless of explicit tag style (!!merge or plain)",
+		document:    mixedMergeTagStyleDocument,
+		expression:  `explode(.)`,
+		expected:    []string{"D0, P[], (!!map)::" + mixedMergeTagStyleExplodedDocument},


This test makes sense to me 👍🏼

mikefarah · 2026-05-14T09:20:04Z

 		log.Debugf("going to applied assignment to LHS: %v with RHS: %v", NodeToString(lhs), NodeToString(candidate))

-		if candidate.Tag == "!!merge" {
+		if candidate.Tag == "!!merge" && !candidate.IsMapKey {


been a long time since I've touched this logic...I'd be wary of making changes. Not sure why this is needed?

I took more of a look, and believe this entire check is no longer required, due to DontFollowAlias: true. Removing it has the same result. I added a test to show what this accomplishes.

W-Floyd · 2026-05-14T15:43:23Z

Testing by adding this to go.mod:

replace go.yaml.in/yaml/v4 v4.0.0-rc.4 => github.com/yaml/go-yaml/v4 v4.0.0-20260512181823-09a581dc5494

…+` traversal

mikefarah · 2026-05-16T12:40:17Z


 		log.Debugf("going to applied assignment to LHS: %v with RHS: %v", NodeToString(lhs), NodeToString(candidate))

-		if candidate.Tag == "!!merge" {


why is this not needed?

Oh, got it, read your comment later in the PR

ingydotnet · 2026-05-16T20:24:46Z

While I believe that yaml/go-yaml#348 is doing the "right" thing, It occurred to me later that the right thing doesn't necessarily mean the "same" thing as v3. I'll take a look and report back on that.

This was referenced May 12, 2026

<< short merge tags are not !!merge modified AND are now quoted, breaking some parsers #2677

Open

Fix !!merge tag regression for https://github.com/mikefarah/yq/issues/2677 yaml/go-yaml#349

Closed

Fix !!merge tag regression for yq #2677 yaml/go-yaml#348

Open

ccoVeille reviewed May 13, 2026

View reviewed changes

W-Floyd added 2 commits May 13, 2026 09:54

Update docs given yaml/go-yaml#348

eb7738f

Fix for mikefarah#2677

b12810b

Depends on yaml/go-yaml#348

W-Floyd force-pushed the fix-2677 branch from 13db651 to 8f34d15 Compare May 13, 2026 16:55

Test for mikefarah#2677

9170377

W-Floyd force-pushed the fix-2677 branch from 8f34d15 to 9170377 Compare May 13, 2026 17:34

W-Floyd changed the title ~~Fix for https://github.com/mikefarah/yq/issues/2677~~ Fix for #2677 May 13, 2026

mikefarah reviewed May 14, 2026

View reviewed changes

Remove redundant check and add test case for explicit !!merge on `*…

1a026f3

…+` traversal

W-Floyd force-pushed the fix-2677 branch from 1044baa to 1a026f3 Compare May 14, 2026 15:57

mikefarah reviewed May 16, 2026

View reviewed changes


		log.Debugf("going to applied assignment to LHS: %v with RHS: %v", NodeToString(lhs), NodeToString(candidate))

		if candidate.Tag == "!!merge" {

Uh oh!

Conversation

W-Floyd commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

W-Floyd commented May 13, 2026

Uh oh!

ccoVeille commented May 13, 2026

Uh oh!

ccoVeille commented May 13, 2026

Uh oh!

W-Floyd commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

W-Floyd May 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

W-Floyd commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ingydotnet commented May 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

W-Floyd commented May 13, 2026 •

edited

Loading

W-Floyd May 14, 2026 •

edited

Loading