From 1fd7b60efec07c8dcf9a1c3182a2a7c1f333db16 Mon Sep 17 00:00:00 2001 From: Ben Tranter Date: Thu, 29 Jun 2017 14:38:48 -0400 Subject: [PATCH] Add more information to basic diff logic --- pkg/components/dashdiffs/formatter_basic.go | 175 +++++++++++++------- 1 file changed, 114 insertions(+), 61 deletions(-) diff --git a/pkg/components/dashdiffs/formatter_basic.go b/pkg/components/dashdiffs/formatter_basic.go index 5b1d5504bc6..af93ff42bf2 100644 --- a/pkg/components/dashdiffs/formatter_basic.go +++ b/pkg/components/dashdiffs/formatter_basic.go @@ -98,13 +98,7 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { blocks := make([]*BasicBlock, 0) for _, line := range lines { - // In order to produce distinct "blocks" when rendering the basic diff, - // we need a way to distinguish between differnt sections of data. - // To do this, we consider the value(s) of each top-level JSON key to - // represent a distinct block for Grafana's JSON data structure, so - // we perform this check to see if we've entered a new "block". If we - // have, we simply append the existing block to the array of blocks. - if b.LastIndent == 2 && line.Indent == 1 && line.Change == ChangeNil { + if b.returnToTopLevelKey(line) { if b.Block != nil { blocks = append(blocks, b.Block) } @@ -114,56 +108,9 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { // check for a change in depth inside the JSON data structures. b.LastIndent = line.Indent - // TODO: why special handling for indent 2? - // Here we - // If the line's indentation is at level 1, then we know it's a top - // level key in the JSON document. As mentioned earlier, we treat these - // specially as they indicate their values belong to distinct blocks. - // - // At level 1, we only record single-line changes, ie, the "added", - // "deleted", "old" or "new" cases, since we know those values aren't - // arrays or maps. We only handle these cases at level 2 or deeper, - // since for those we either output a "change" or "summary". This is - // done for formatting reasons only, so we have logical "blocks" to - // display. if line.Indent == 1 { - switch line.Change { - case ChangeNil: - if line.Change == ChangeNil { - if line.Key != "" { - b.Block = &BasicBlock{ - Title: line.Key, - Change: line.Change, - } - } - } - - case ChangeAdded, ChangeDeleted: - blocks = append(blocks, &BasicBlock{ - Title: line.Key, - Change: line.Change, - New: line.Val, - LineStart: line.LineNum, - }) - - case ChangeOld: - b.Block = &BasicBlock{ - Title: line.Key, - Old: line.Val, - Change: line.Change, - LineStart: line.LineNum, - } - - case ChangeNew: - b.Block.New = line.Val - b.Block.LineEnd = line.LineNum - - // For every "old" change there is a corresponding "new", which - // is why we wait until we detect the "new" change before - // appending the change. - blocks = append(blocks, b.Block) - default: - // ok + if block, ok := b.handleTopLevelChange(line); ok { + blocks = append(blocks, block) } } @@ -182,8 +129,8 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { // finding the change, we append it to the current block, and begin // performing comparisons again. if line.Indent > 1 { - // Ensure a single line change - if line.Key != "" && line.Val != nil && !b.writing { + // check to ensure a single line change + if b.isSingleLineChange(line) { switch line.Change { case ChangeAdded, ChangeDeleted: @@ -211,13 +158,31 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { //ok } + // otherwise, we're dealing with a change at a deeper level. We + // know there's a change somewhere in the JSON tree, but we + // don't know exactly where, so we go deeper. } else { + + // if the change is anything but unchanged, continue processing + // + // we keep "narrowing" the key as we go deeper, in order to + // correctly report the key name for changes found within an + // object or array. if line.Change != ChangeUnchanged { if line.Key != "" { b.narrow = line.Key b.keysIdent = line.Indent } + // if the change isn't nil, and we're not already writing + // out a change, then we've found something. + // + // First, try to determine the title of the embedded JSON + // object. If it's an empty string, then we're in an object + // or array, so we default to using the "narrowed" key. + // + // We also start recording the basic summary, until we find + // the next `ChangeUnchanged`. if line.Change != ChangeNil { if !b.writing { b.writing = true @@ -237,6 +202,17 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { } } } + // if we find a `ChangeUnchanged`, we do one of two things: + // + // - if we're recording a change already, then we know + // we've come to the end of that change block, so we write + // that change out be recording the line number of where + // that change ends, and append it to the current block's + // summary. + // + // - if we're not recording a change, then we do nothing, + // since the BasicDiff doesn't report on unchanged JSON + // values. } else { if b.writing { b.writing = false @@ -251,6 +227,81 @@ func (b *BasicDiff) Basic(lines []*JSONLine) []*BasicBlock { return blocks } +// returnToTopLevelKey indicates that we've moved from a key at one level deep +// in the JSON document to a top level key. +// +// In order to produce distinct "blocks" when rendering the basic diff, +// we need a way to distinguish between differnt sections of data. +// To do this, we consider the value(s) of each top-level JSON key to +// represent a distinct block for Grafana's JSON data structure, so +// we perform this check to see if we've entered a new "block". If we +// have, we simply append the existing block to the array of blocks. +func (b *BasicDiff) returnToTopLevelKey(line *JSONLine) bool { + return b.LastIndent == 2 && line.Indent == 1 && line.Change == ChangeNil +} + +// handleTopLevelChange handles a change on one of the top-level keys on a JSON +// document. +// +// If the line's indentation is at level 1, then we know it's a top +// level key in the JSON document. As mentioned earlier, we treat these +// specially as they indicate their values belong to distinct blocks. +// +// At level 1, we only record single-line changes, ie, the "added", +// "deleted", "old" or "new" cases, since we know those values aren't +// arrays or maps. We only handle these cases at level 2 or deeper, +// since for those we either output a "change" or "summary". This is +// done for formatting reasons only, so we have logical "blocks" to +// display. +func (b *BasicDiff) handleTopLevelChange(line *JSONLine) (*BasicBlock, bool) { + switch line.Change { + case ChangeNil: + if line.Change == ChangeNil { + if line.Key != "" { + b.Block = &BasicBlock{ + Title: line.Key, + Change: line.Change, + } + } + } + + case ChangeAdded, ChangeDeleted: + return &BasicBlock{ + Title: line.Key, + Change: line.Change, + New: line.Val, + LineStart: line.LineNum, + }, true + + case ChangeOld: + b.Block = &BasicBlock{ + Title: line.Key, + Old: line.Val, + Change: line.Change, + LineStart: line.LineNum, + } + + case ChangeNew: + b.Block.New = line.Val + b.Block.LineEnd = line.LineNum + + // For every "old" change there is a corresponding "new", which + // is why we wait until we detect the "new" change before + // appending the change. + return b.Block, true + default: + // ok + } + + return nil, false +} + +// isSingleLineChange ensures we're iterating over a single line change (ie, +// either a single line or a old-new value pair was changed in the JSON file). +func (b *BasicDiff) isSingleLineChange(line *JSONLine) bool { + return line.Key != "" && line.Val != nil && !b.writing +} + // encStateMap is used in the template helper var ( encStateMap = map[ChangeType]string{ @@ -273,7 +324,9 @@ var ( ) var ( - // tplBlock is the whole thing + // tplBlock is the container for the basic diff. It iterates over each + // basic block, expanding each "change" and "summary" belonging to every + // block. tplBlock = `{{ define "block" -}} {{ range . }}
@@ -319,7 +372,7 @@ var ( {{ end }} {{ end }}` - // tplChange is the template for changes + // tplChange is the template for basic changes. tplChange = `{{ define "change" -}}
  • @@ -346,7 +399,7 @@ var (
    • {{ end }}` - // tplSummary is for basis summaries + // tplSummary is for basic summaries. tplSummary = `{{ define "summary" -}}