From 8a6af1720d5e189e7853fd63d7173fbd9e689f30 Mon Sep 17 00:00:00 2001 From: Mike Farah Date: Mon, 30 Dec 2019 11:21:21 +1300 Subject: [PATCH] Fixed modify array issue! --- commands_test.go | 9 ++++++ pkg/yqlib/data_navigator.go | 32 +++++++++++++------- pkg/yqlib/delete_navigation_strategy.go | 2 +- pkg/yqlib/lib.go | 18 +++++++++++ pkg/yqlib/navigation_strategy.go | 40 +++++++++++++++++++------ yq.go | 22 ++------------ 6 files changed, 82 insertions(+), 41 deletions(-) diff --git a/commands_test.go b/commands_test.go index 9c07897e..a7c9f84f 100644 --- a/commands_test.go +++ b/commands_test.go @@ -103,6 +103,15 @@ func TestReadWithKeyAndValueCmd(t *testing.T) { test.AssertResult(t, "b.c: 2\n", result.Output) } +func TestReadArrayCmd(t *testing.T) { + cmd := getRootCommand() + result := test.RunCmd(cmd, "read -p kv examples/sample.yaml b.e.1.name") + if result.Error != nil { + t.Error(result.Error) + } + test.AssertResult(t, "b.e.1.name: sam\n", result.Output) +} + func TestReadWithKeyCmd(t *testing.T) { cmd := getRootCommand() result := test.RunCmd(cmd, "read -p k examples/sample.yaml b.c") diff --git a/pkg/yqlib/data_navigator.go b/pkg/yqlib/data_navigator.go index d47e8011..e92c4c2a 100644 --- a/pkg/yqlib/data_navigator.go +++ b/pkg/yqlib/data_navigator.go @@ -37,8 +37,7 @@ func (n *navigator) doTraverse(value *yaml.Node, head string, path []string, pat DebugNode(value) return n.recurse(value, path[0], path[1:], pathStack) } - log.Debug("should I visit?") - return n.navigationStrategy.Visit(NodeContext{value, head, path, pathStack}) + return n.navigationStrategy.Visit(NewNodeContext(value, head, path, pathStack)) } func (n *navigator) getOrReplace(original *yaml.Node, expectedKind yaml.Kind) *yaml.Node { @@ -66,7 +65,7 @@ func (n *navigator) recurse(value *yaml.Node, head string, tail []string, pathSt case yaml.AliasNode: log.Debug("its an alias!") DebugNode(value.Alias) - if n.navigationStrategy.FollowAlias(NodeContext{value, head, tail, pathStack}) == true { + if n.navigationStrategy.FollowAlias(NewNodeContext(value, head, tail, pathStack)) == true { log.Debug("following the alias") return n.recurse(value.Alias, head, tail, pathStack) } @@ -79,15 +78,21 @@ func (n *navigator) recurse(value *yaml.Node, head string, tail []string, pathSt func (n *navigator) recurseMap(value *yaml.Node, head string, tail []string, pathStack []interface{}) error { traversedEntry := false errorVisiting := n.visitMatchingEntries(value, head, tail, pathStack, func(contents []*yaml.Node, indexInMap int) error { - - log.Debug("should I traverse? %v", head) - DebugNode(value) + log.Debug("recurseMap: visitMatchingEntries") + n.navigationStrategy.DebugVisitedNodes() newPathStack := append(pathStack, contents[indexInMap].Value) - if n.navigationStrategy.ShouldTraverse(NodeContext{contents[indexInMap+1], head, tail, newPathStack}, contents[indexInMap].Value) == true { - log.Debug("yep!") + log.Debug("appended %v", contents[indexInMap].Value) + n.navigationStrategy.DebugVisitedNodes() + log.Debug("should I traverse? %v, %v", head, PathStackToString(newPathStack)) + DebugNode(value) + if n.navigationStrategy.ShouldTraverse(NewNodeContext(contents[indexInMap+1], head, tail, newPathStack), contents[indexInMap].Value) == true { + log.Debug("recurseMap: Going to traverse") traversedEntry = true contents[indexInMap+1] = n.getOrReplace(contents[indexInMap+1], guessKind(tail, contents[indexInMap+1].Kind)) - return n.doTraverse(contents[indexInMap+1], head, tail, newPathStack) + errorTraversing := n.doTraverse(contents[indexInMap+1], head, tail, newPathStack) + log.Debug("recurseMap: Finished traversing") + n.navigationStrategy.DebugVisitedNodes() + return errorTraversing } else { log.Debug("nope not traversing") } @@ -98,7 +103,7 @@ func (n *navigator) recurseMap(value *yaml.Node, head string, tail []string, pat return errorVisiting } - if traversedEntry == true || head == "*" || n.navigationStrategy.AutoCreateMap(NodeContext{value, head, tail, pathStack}) == false { + if traversedEntry == true || head == "*" || n.navigationStrategy.AutoCreateMap(NewNodeContext(value, head, tail, pathStack)) == false { return nil } @@ -117,7 +122,9 @@ func (n *navigator) visitDirectMatchingEntries(node *yaml.Node, head string, tai var contents = node.Content for index := 0; index < len(contents); index = index + 2 { content := contents[index] + log.Debug("index %v, checking %v, %v", index, content.Value, content.Tag) + n.navigationStrategy.DebugVisitedNodes() errorVisiting := visit(contents, index) if errorVisiting != nil { return errorVisiting @@ -136,7 +143,7 @@ func (n *navigator) visitMatchingEntries(node *yaml.Node, head string, tail []st // if we don't find a match directly on this node first. errorVisitedDirectEntries := n.visitDirectMatchingEntries(node, head, tail, pathStack, visit) - if errorVisitedDirectEntries != nil || n.navigationStrategy.FollowAlias(NodeContext{node, head, tail, pathStack}) == false { + if errorVisitedDirectEntries != nil || n.navigationStrategy.FollowAlias(NewNodeContext(node, head, tail, pathStack)) == false { return errorVisitedDirectEntries } return n.visitAliases(contents, head, tail, pathStack, visit) @@ -220,5 +227,8 @@ func (n *navigator) recurseArray(value *yaml.Node, head string, tail []string, p return nil } value.Content[index] = n.getOrReplace(value.Content[index], guessKind(tail, value.Content[index].Kind)) + + // THERES SOMETHING WRONG HERE, ./yq read -p kv examples/sample.yaml b.e.1.* + // THERES SOMETHING WRONG HERE, ./yq read -p kv examples/sample.yaml b.e.1.name return n.doTraverse(value.Content[index], head, tail, append(pathStack, index)) } diff --git a/pkg/yqlib/delete_navigation_strategy.go b/pkg/yqlib/delete_navigation_strategy.go index 8d221c4a..b4f95c3a 100644 --- a/pkg/yqlib/delete_navigation_strategy.go +++ b/pkg/yqlib/delete_navigation_strategy.go @@ -38,7 +38,7 @@ func deleteFromMap(pathParser PathParser, contents []*yaml.Node, pathStack []int for index := 0; index < len(contents); index = index + 2 { keyNode := contents[index] valueNode := contents[index+1] - if pathParser.MatchesNextPathElement(NodeContext{keyNode, pathElementToDelete, []string{}, pathStack}, keyNode.Value) == false { + if pathParser.MatchesNextPathElement(NewNodeContext(keyNode, pathElementToDelete, []string{}, pathStack), keyNode.Value) == false { log.Debug("adding node %v", keyNode.Value) newContents = append(newContents, keyNode, valueNode) } else { diff --git a/pkg/yqlib/lib.go b/pkg/yqlib/lib.go index 74ed5517..2217a1b5 100644 --- a/pkg/yqlib/lib.go +++ b/pkg/yqlib/lib.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "strconv" + "strings" logging "gopkg.in/op/go-logging.v1" yaml "gopkg.in/yaml.v3" @@ -30,6 +31,23 @@ func DebugNode(value *yaml.Node) { } } +func PathStackToString(pathStack []interface{}) string { + var sb strings.Builder + for index, path := range pathStack { + switch path.(type) { + case int: + sb.WriteString(fmt.Sprintf("[%v]", path)) + default: + sb.WriteString(fmt.Sprintf("%v", path)) + } + + if index < len(pathStack)-1 { + sb.WriteString(".") + } + } + return sb.String() +} + func guessKind(tail []string, guess yaml.Kind) yaml.Kind { log.Debug("tail %v", tail) if len(tail) == 0 && guess == 0 { diff --git a/pkg/yqlib/navigation_strategy.go b/pkg/yqlib/navigation_strategy.go index 55f4acb7..85dd5b21 100644 --- a/pkg/yqlib/navigation_strategy.go +++ b/pkg/yqlib/navigation_strategy.go @@ -13,6 +13,20 @@ type NodeContext struct { PathStack []interface{} } +func NewNodeContext(node *yaml.Node, head string, tail []string, pathStack []interface{}) NodeContext { + newTail := make([]string, len(tail)) + copy(newTail, tail) + + newPathStack := make([]interface{}, len(pathStack)) + copy(newPathStack, pathStack) + return NodeContext{ + Node: node, + Head: head, + Tail: newTail, + PathStack: newPathStack, + } +} + type NavigationStrategy interface { FollowAlias(nodeContext NodeContext) bool AutoCreateMap(nodeContext NodeContext) bool @@ -21,6 +35,7 @@ type NavigationStrategy interface { // we use it to match against the pathExpression in head. ShouldTraverse(nodeContext NodeContext, nodeKey string) bool GetVisitedNodes() []*NodeContext + DebugVisitedNodes() } type NavigationStrategyImpl struct { @@ -78,19 +93,29 @@ func (ns *NavigationStrategyImpl) shouldVisit(nodeContext NodeContext) bool { } func (ns *NavigationStrategyImpl) Visit(nodeContext NodeContext) error { + log.Debug("Visit?, %v, %v", nodeContext.Head, PathStackToString(nodeContext.PathStack)) + DebugNode(nodeContext.Node) if ns.shouldVisit(nodeContext) { + log.Debug("yep, visiting") + // pathStack array must be + // copied, as append() may sometimes reuse and modify the array ns.visitedNodes = append(ns.visitedNodes, &nodeContext) - log.Debug("adding to visited nodes, %v", nodeContext.Head) + ns.DebugVisitedNodes() return ns.visit(nodeContext) } + log.Debug("nope, skip it") return nil } -func (ns *NavigationStrategyImpl) alreadyVisited(pathStack []interface{}) bool { - log.Debug("looking for pathStack") - for _, val := range pathStack { - log.Debug("\t %v", val) +func (ns *NavigationStrategyImpl) DebugVisitedNodes() { + log.Debug("%v", ns.visitedNodes) + for _, candidate := range ns.visitedNodes { + log.Debug(" - %v", PathStackToString(candidate.PathStack)) } +} + +func (ns *NavigationStrategyImpl) alreadyVisited(pathStack []interface{}) bool { + log.Debug("checking already visited pathStack: %v", PathStackToString(pathStack)) for _, candidate := range ns.visitedNodes { candidatePathStack := candidate.PathStack if patchStacksMatch(candidatePathStack, pathStack) { @@ -104,10 +129,7 @@ func (ns *NavigationStrategyImpl) alreadyVisited(pathStack []interface{}) bool { } func patchStacksMatch(path1 []interface{}, path2 []interface{}) bool { - log.Debug("checking against path") - for _, val := range path1 { - log.Debug("\t %v", val) - } + log.Debug("checking against path: %v", PathStackToString(path1)) if len(path1) != len(path2) { return false diff --git a/yq.go b/yq.go index 7d1c72b9..59bb74a2 100644 --- a/yq.go +++ b/yq.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "os" "strconv" - "strings" "github.com/mikefarah/yq/v3/pkg/yqlib" @@ -306,23 +305,6 @@ func appendDocument(originalMatchingNodes []*yqlib.NodeContext, dataBucket yaml. return append(originalMatchingNodes, matchingNodes...), nil } -func pathToString(pathStack []interface{}) string { - var sb strings.Builder - for index, path := range pathStack { - switch path.(type) { - case int: - sb.WriteString(fmt.Sprintf("[%v]", path)) - default: - sb.WriteString(fmt.Sprintf("%v", path)) - } - - if index < len(pathStack)-1 { - sb.WriteString(".") - } - } - return sb.String() -} - func printValue(node *yaml.Node, cmd *cobra.Command) error { if node.Kind == yaml.ScalarNode { cmd.Print(node.Value) @@ -346,7 +328,7 @@ func printResults(matchingNodes []*yqlib.NodeContext, cmd *cobra.Command) error for index, mappedDoc := range matchingNodes { switch printMode { case "k": - cmd.Print(pathToString(mappedDoc.PathStack)) + cmd.Print(yqlib.PathStackToString(mappedDoc.PathStack)) if index < len(matchingNodes)-1 { cmd.Print("\n") } @@ -354,7 +336,7 @@ func printResults(matchingNodes []*yqlib.NodeContext, cmd *cobra.Command) error // put it into a node and print that. var parentNode = yaml.Node{Kind: yaml.MappingNode} parentNode.Content = make([]*yaml.Node, 2) - parentNode.Content[0] = &yaml.Node{Kind: yaml.ScalarNode, Value: pathToString(mappedDoc.PathStack)} + parentNode.Content[0] = &yaml.Node{Kind: yaml.ScalarNode, Value: yqlib.PathStackToString(mappedDoc.PathStack)} parentNode.Content[1] = mappedDoc.Node if err := printValue(&parentNode, cmd); err != nil { return err