Prevent NPE in CSV diff rendering when column removed (#17018) (#17377)

Backport of #17018

Fixes #16837 if a column is deleted.
This commit is contained in:
Richard Mahn 2021-10-20 14:55:34 -06:00 committed by GitHub
parent 79f0b1a50b
commit befb6bea22
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 355 additions and 140 deletions

View File

@ -32,6 +32,9 @@ func CreateReaderAndGuessDelimiter(rd io.Reader) (*stdcsv.Reader, error) {
var data = make([]byte, 1e4)
size, err := rd.Read(data)
if err != nil {
if err == io.EOF {
return CreateReader(bytes.NewReader([]byte{}), rune(',')), nil
}
return nil, err
}

File diff suppressed because it is too large Load Diff

View File

@ -19,9 +19,9 @@ func TestCSVDiff(t *testing.T) {
diff string
base string
head string
cells [][2]TableDiffCellType
cells [][]TableDiffCellType
}{
// case 0
// case 0 - initial commit of a csv
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
@ -29,11 +29,14 @@ func TestCSVDiff(t *testing.T) {
@@ -0,0 +1,2 @@
+col1,col2
+a,a`,
base: "",
head: "col1,col2\na,a",
cells: [][2]TableDiffCellType{{TableDiffCellAdd, TableDiffCellAdd}, {TableDiffCellAdd, TableDiffCellAdd}},
base: "",
head: `col1,col2
a,a`,
cells: [][]TableDiffCellType{
{TableDiffCellAdd, TableDiffCellAdd},
{TableDiffCellAdd, TableDiffCellAdd}},
},
// case 1
// case 1 - adding 1 row at end
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
@ -43,11 +46,17 @@ func TestCSVDiff(t *testing.T) {
-a,a
+a,a
+b,b`,
base: "col1,col2\na,a",
head: "col1,col2\na,a\nb,b",
cells: [][2]TableDiffCellType{{TableDiffCellEqual, TableDiffCellEqual}, {TableDiffCellEqual, TableDiffCellEqual}, {TableDiffCellAdd, TableDiffCellAdd}},
base: `col1,col2
a,a`,
head: `col1,col2
a,a
b,b`,
cells: [][]TableDiffCellType{
{TableDiffCellUnchanged, TableDiffCellUnchanged}, {TableDiffCellUnchanged, TableDiffCellUnchanged},
{TableDiffCellAdd, TableDiffCellAdd},
},
},
// case 2
// case 2 - row deleted
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
@ -56,11 +65,17 @@ func TestCSVDiff(t *testing.T) {
col1,col2
-a,a
b,b`,
base: "col1,col2\na,a\nb,b",
head: "col1,col2\nb,b",
cells: [][2]TableDiffCellType{{TableDiffCellEqual, TableDiffCellEqual}, {TableDiffCellDel, TableDiffCellDel}, {TableDiffCellEqual, TableDiffCellEqual}},
base: `col1,col2
a,a
b,b`,
head: `col1,col2
b,b`,
cells: [][]TableDiffCellType{
{TableDiffCellUnchanged, TableDiffCellUnchanged}, {TableDiffCellDel, TableDiffCellDel},
{TableDiffCellUnchanged, TableDiffCellUnchanged},
},
},
// case 3
// case 3 - row changed
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
@ -69,11 +84,16 @@ func TestCSVDiff(t *testing.T) {
col1,col2
-b,b
+b,c`,
base: "col1,col2\nb,b",
head: "col1,col2\nb,c",
cells: [][2]TableDiffCellType{{TableDiffCellEqual, TableDiffCellEqual}, {TableDiffCellEqual, TableDiffCellChanged}},
base: `col1,col2
b,b`,
head: `col1,col2
b,c`,
cells: [][]TableDiffCellType{
{TableDiffCellUnchanged, TableDiffCellUnchanged},
{TableDiffCellUnchanged, TableDiffCellChanged},
},
},
// case 4
// case 4 - all deleted
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
@ -81,9 +101,88 @@ func TestCSVDiff(t *testing.T) {
@@ -1,2 +0,0 @@
-col1,col2
-b,c`,
base: "col1,col2\nb,c",
head: "",
cells: [][2]TableDiffCellType{{TableDiffCellDel, TableDiffCellDel}, {TableDiffCellDel, TableDiffCellDel}},
base: `col1,col2
b,c`,
head: "",
cells: [][]TableDiffCellType{
{TableDiffCellDel, TableDiffCellDel},
{TableDiffCellDel, TableDiffCellDel},
},
},
// case 5 - renames first column
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
+++ b/unittest.csv
@@ -1,3 +1,3 @@
-col1,col2,col3
+cola,col2,col3
a,b,c`,
base: `col1,col2,col3
a,b,c`,
head: `cola,col2,col3
a,b,c`,
cells: [][]TableDiffCellType{
{TableDiffCellDel, TableDiffCellAdd, TableDiffCellUnchanged, TableDiffCellUnchanged},
{TableDiffCellDel, TableDiffCellAdd, TableDiffCellUnchanged, TableDiffCellUnchanged},
},
},
// case 6 - inserts a column after first, deletes last column
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
+++ b/unittest.csv
@@ -1,2 +1,2 @@
-col1,col2,col3
-a,b,c
+col1,col1a,col2
+a,d,b`,
base: `col1,col2,col3
a,b,c`,
head: `col1,col1a,col2
a,d,b`,
cells: [][]TableDiffCellType{
{TableDiffCellUnchanged, TableDiffCellAdd, TableDiffCellDel, TableDiffCellMovedUnchanged},
{TableDiffCellUnchanged, TableDiffCellAdd, TableDiffCellDel, TableDiffCellMovedUnchanged},
},
},
// case 7 - deletes first column, inserts column after last
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
+++ b/unittest.csv
@@ -1,2 +1,2 @@
-col1,col2,col3
-a,b,c
+col2,col3,col4
+b,c,d`,
base: `col1,col2,col3
a,b,c`,
head: `col2,col3,col4
b,c,d`,
cells: [][]TableDiffCellType{
{TableDiffCellDel, TableDiffCellUnchanged, TableDiffCellUnchanged, TableDiffCellAdd},
{TableDiffCellDel, TableDiffCellUnchanged, TableDiffCellUnchanged, TableDiffCellAdd},
},
},
// case 8 - two columns deleted, 2 added
{
diff: `diff --git a/unittest.csv b/unittest.csv
--- a/unittest.csv
+++ b/unittest.csv
@@ -1,2 +1,2 @@
-col1,col2,col
-a,b,c
+col3,col4,col5
+c,d,e`,
base: `col1,col2,col3
a,b,c`,
head: `col3,col4,col5
c,d,e`,
cells: [][]TableDiffCellType{
{TableDiffCellDel, TableDiffCellMovedUnchanged, TableDiffCellDel, TableDiffCellAdd, TableDiffCellAdd},
{TableDiffCellDel, TableDiffCellMovedUnchanged, TableDiffCellDel, TableDiffCellAdd, TableDiffCellAdd},
},
},
}
@ -116,7 +215,7 @@ func TestCSVDiff(t *testing.T) {
assert.Len(t, section.Rows, len(c.cells), "case %d: should be %d rows", n, len(c.cells))
for i, row := range section.Rows {
assert.Len(t, row.Cells, 2, "case %d: row %d should have two cells", n, i)
assert.Len(t, row.Cells, len(c.cells[i]), "case %d: row %d should have %d cells", n, i, len(c.cells[i]))
for j, cell := range row.Cells {
assert.Equal(t, c.cells[i][j], cell.Type, "case %d: row %d cell %d should be equal", n, i, j)
}

View File

@ -12,12 +12,18 @@
{{if and (eq $i 0) (eq $j 0)}}
<th class="line-num">{{.RowIdx}}</th>
{{range $j, $cell := $row.Cells}}
{{if eq $cell.Type 2}}
{{if not $cell}}
<th></th>
{{else if eq $cell.Type 2}}
<th class="modified"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></th>
{{else if eq $cell.Type 3}}
<th class="added"><span class="added-code">{{.LeftCell}}</span></th>
<th class="added"><span class="added-code">{{.RightCell}}</span></th>
{{else if eq $cell.Type 4}}
<th class="removed"><span class="removed-code">{{.LeftCell}}</span></th>
{{else if eq $cell.Type 5}}
<th class="moved">{{.RightCell}}</th>
{{else if eq $cell.Type 6}}
<th class="moved"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></th>
{{else}}
<th>{{.RightCell}}</th>
{{end}}
@ -25,12 +31,18 @@
{{else}}
<td class="line-num">{{if .RowIdx}}{{.RowIdx}}{{end}}</td>
{{range $j, $cell := $row.Cells}}
{{if eq $cell.Type 2}}
{{if not $cell}}
<td></td>
{{else if eq $cell.Type 2}}
<td class="modified"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></td>
{{else if eq $cell.Type 3}}
<td class="added"><span class="added-code">{{.LeftCell}}</span></td>
<td class="added"><span class="added-code">{{.RightCell}}</span></td>
{{else if eq $cell.Type 4}}
<td class="removed"><span class="removed-code">{{.LeftCell}}</span></td>
{{else if eq $cell.Type 5}}
<td class="moved">{{.RightCell}}</td>
{{else if eq $cell.Type 6}}
<td class="moved"><span class="removed-code">{{.LeftCell}}</span> <span class="added-code">{{.RightCell}}</span></td>
{{else}}
<td>{{.RightCell}}</td>
{{end}}

View File

@ -76,8 +76,10 @@
--color-diff-removed-word-bg: #fdb8c0;
--color-diff-added-word-bg: #acf2bd;
--color-diff-removed-row-bg: #ffeef0;
--color-diff-moved-row-bg: #f1f8d1;
--color-diff-added-row-bg: #e6ffed;
--color-diff-removed-row-border: #f1c0c0;
--color-diff-moved-row-border: #d0e27f;
--color-diff-added-row-border: #e6ffed;
--color-diff-inactive: #f2f2f2;
/* target-based colors */

View File

@ -1500,6 +1500,12 @@
background-color: var(--color-diff-removed-row-bg) !important;
}
td.moved,
th.moved,
tr.moved {
background-color: var(--color-diff-moved-row-bg) !important;
}
tbody.section {
border-top: 2px solid var(--color-secondary);
}

View File

@ -71,8 +71,10 @@
--color-diff-removed-word-bg: #6f3333;
--color-diff-added-word-bg: #3c653c;
--color-diff-removed-row-bg: #3c2626;
--color-diff-moved-row-bg: #818044;
--color-diff-added-row-bg: #283e2d;
--color-diff-removed-row-border: #634343;
--color-diff-moved-row-border: #bcca6f;
--color-diff-added-row-border: #314a37;
--color-diff-inactive: #353846;
/* target-based colors */