Add more information to basic diff logic

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,
+		if b.returnToTopLevelKey(line) {
-		// 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.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 {
+			if block, ok := b.handleTopLevelChange(line); ok {
-			case ChangeNil:
+				blocks = append(blocks, block)
-				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
 			}
 		}
 
@@ -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
+			// check to ensure a single line change
-			if line.Key != "" && line.Val != nil && !b.writing {
+			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 . }}
 <div class="diff-group">
@@ -319,7 +372,7 @@ var (
 {{ end }}
 {{ end }}`
 
-	// tplChange is the template for changes
+	// tplChange is the template for basic changes.
 	tplChange = `{{ define "change" -}}
 <li class="diff-change-group">
 	<span class="bullet-position-container">
@@ -346,7 +399,7 @@ var (
 </li>
 {{ end }}`
 
-	// tplSummary is for basis summaries
+	// tplSummary is for basic summaries.
 	tplSummary = `{{ define "summary" -}}
 <div class="diff-group-name">
 	<i class="diff-circle diff-circle-{{ getChange .Change }} fa fa-circle-o diff-list-circle"></i>