From 75ec9ca946868571dc2c1adcaca4c8c60ebfa956 Mon Sep 17 00:00:00 2001 From: Daylon Wilkins Date: Wed, 22 Feb 2023 05:50:39 -0800 Subject: [PATCH] Line-based diffs for modified multiline rows --- go/Godeps/LICENSES | 209 ++++++++++++++++++ go/cmd/dolt/commands/diff.go | 48 ++-- go/go.mod | 3 +- go/go.sum | 2 + go/libraries/doltcore/diff/diff.go | 13 ++ .../table/typed/json/json_diff_writer.go | 4 + .../untyped/sqlexport/sql_diff_writer.go | 6 + .../fixedwidth_conflict_tablewriter.go | 2 +- .../tabular/fixedwidth_diff_tablewriter.go | 109 ++++++++- .../untyped/tabular/fixedwidth_tablewriter.go | 80 ++++--- .../tabular/fixedwidth_tablewriter_test.go | 40 ++++ .../table/untyped/tabular/stringwidth.go | 36 ++- integration-tests/bats/diff.bats | 172 +++++++++++++- 13 files changed, 670 insertions(+), 54 deletions(-) diff --git a/go/Godeps/LICENSES b/go/Godeps/LICENSES index 5b038f407c..e3afe4347e 100644 --- a/go/Godeps/LICENSES +++ b/go/Godeps/LICENSES @@ -4522,6 +4522,215 @@ SOFTWARE. = LICENSE 24dd21670b04d159a0d6c283884da9d23f8761460c300d4702ccffa5 = ================================================================================ +================================================================================ += github.com/kylelemons/godebug licensed under: = + + + Apache License + Version 2.0, January 2004 + http://www.apache.org/licenses/ + + TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION + + 1. Definitions. + + "License" shall mean the terms and conditions for use, reproduction, + and distribution as defined by Sections 1 through 9 of this document. + + "Licensor" shall mean the copyright owner or entity authorized by + the copyright owner that is granting the License. + + "Legal Entity" shall mean the union of the acting entity and all + other entities that control, are controlled by, or are under common + control with that entity. For the purposes of this definition, + "control" means (i) the power, direct or indirect, to cause the + direction or management of such entity, whether by contract or + otherwise, or (ii) ownership of fifty percent (50%) or more of the + outstanding shares, or (iii) beneficial ownership of such entity. + + "You" (or "Your") shall mean an individual or Legal Entity + exercising permissions granted by this License. + + "Source" form shall mean the preferred form for making modifications, + including but not limited to software source code, documentation + source, and configuration files. + + "Object" form shall mean any form resulting from mechanical + transformation or translation of a Source form, including but + not limited to compiled object code, generated documentation, + and conversions to other media types. + + "Work" shall mean the work of authorship, whether in Source or + Object form, made available under the License, as indicated by a + copyright notice that is included in or attached to the work + (an example is provided in the Appendix below). + + "Derivative Works" shall mean any work, whether in Source or Object + form, that is based on (or derived from) the Work and for which the + editorial revisions, annotations, elaborations, or other modifications + represent, as a whole, an original work of authorship. For the purposes + of this License, Derivative Works shall not include works that remain + separable from, or merely link (or bind by name) to the interfaces of, + the Work and Derivative Works thereof. + + "Contribution" shall mean any work of authorship, including + the original version of the Work and any modifications or additions + to that Work or Derivative Works thereof, that is intentionally + submitted to Licensor for inclusion in the Work by the copyright owner + or by an individual or Legal Entity authorized to submit on behalf of + the copyright owner. For the purposes of this definition, "submitted" + means any form of electronic, verbal, or written communication sent + to the Licensor or its representatives, including but not limited to + communication on electronic mailing lists, source code control systems, + and issue tracking systems that are managed by, or on behalf of, the + Licensor for the purpose of discussing and improving the Work, but + excluding communication that is conspicuously marked or otherwise + designated in writing by the copyright owner as "Not a Contribution." + + "Contributor" shall mean Licensor and any individual or Legal Entity + on behalf of whom a Contribution has been received by Licensor and + subsequently incorporated within the Work. + + 2. Grant of Copyright License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + copyright license to reproduce, prepare Derivative Works of, + publicly display, publicly perform, sublicense, and distribute the + Work and such Derivative Works in Source or Object form. + + 3. Grant of Patent License. Subject to the terms and conditions of + this License, each Contributor hereby grants to You a perpetual, + worldwide, non-exclusive, no-charge, royalty-free, irrevocable + (except as stated in this section) patent license to make, have made, + use, offer to sell, sell, import, and otherwise transfer the Work, + where such license applies only to those patent claims licensable + by such Contributor that are necessarily infringed by their + Contribution(s) alone or by combination of their Contribution(s) + with the Work to which such Contribution(s) was submitted. If You + institute patent litigation against any entity (including a + cross-claim or counterclaim in a lawsuit) alleging that the Work + or a Contribution incorporated within the Work constitutes direct + or contributory patent infringement, then any patent licenses + granted to You under this License for that Work shall terminate + as of the date such litigation is filed. + + 4. Redistribution. You may reproduce and distribute copies of the + Work or Derivative Works thereof in any medium, with or without + modifications, and in Source or Object form, provided that You + meet the following conditions: + + (a) You must give any other recipients of the Work or + Derivative Works a copy of this License; and + + (b) You must cause any modified files to carry prominent notices + stating that You changed the files; and + + (c) You must retain, in the Source form of any Derivative Works + that You distribute, all copyright, patent, trademark, and + attribution notices from the Source form of the Work, + excluding those notices that do not pertain to any part of + the Derivative Works; and + + (d) If the Work includes a "NOTICE" text file as part of its + distribution, then any Derivative Works that You distribute must + include a readable copy of the attribution notices contained + within such NOTICE file, excluding those notices that do not + pertain to any part of the Derivative Works, in at least one + of the following places: within a NOTICE text file distributed + as part of the Derivative Works; within the Source form or + documentation, if provided along with the Derivative Works; or, + within a display generated by the Derivative Works, if and + wherever such third-party notices normally appear. The contents + of the NOTICE file are for informational purposes only and + do not modify the License. You may add Your own attribution + notices within Derivative Works that You distribute, alongside + or as an addendum to the NOTICE text from the Work, provided + that such additional attribution notices cannot be construed + as modifying the License. + + You may add Your own copyright statement to Your modifications and + may provide additional or different license terms and conditions + for use, reproduction, or distribution of Your modifications, or + for any such Derivative Works as a whole, provided Your use, + reproduction, and distribution of the Work otherwise complies with + the conditions stated in this License. + + 5. Submission of Contributions. Unless You explicitly state otherwise, + any Contribution intentionally submitted for inclusion in the Work + by You to the Licensor shall be under the terms and conditions of + this License, without any additional terms or conditions. + Notwithstanding the above, nothing herein shall supersede or modify + the terms of any separate license agreement you may have executed + with Licensor regarding such Contributions. + + 6. Trademarks. This License does not grant permission to use the trade + names, trademarks, service marks, or product names of the Licensor, + except as required for reasonable and customary use in describing the + origin of the Work and reproducing the content of the NOTICE file. + + 7. Disclaimer of Warranty. Unless required by applicable law or + agreed to in writing, Licensor provides the Work (and each + Contributor provides its Contributions) on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + implied, including, without limitation, any warranties or conditions + of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A + PARTICULAR PURPOSE. You are solely responsible for determining the + appropriateness of using or redistributing the Work and assume any + risks associated with Your exercise of permissions under this License. + + 8. Limitation of Liability. In no event and under no legal theory, + whether in tort (including negligence), contract, or otherwise, + unless required by applicable law (such as deliberate and grossly + negligent acts) or agreed to in writing, shall any Contributor be + liable to You for damages, including any direct, indirect, special, + incidental, or consequential damages of any character arising as a + result of this License or out of the use or inability to use the + Work (including but not limited to damages for loss of goodwill, + work stoppage, computer failure or malfunction, or any and all + other commercial damages or losses), even if such Contributor + has been advised of the possibility of such damages. + + 9. Accepting Warranty or Additional Liability. While redistributing + the Work or Derivative Works thereof, You may choose to offer, + and charge a fee for, acceptance of support, warranty, indemnity, + or other liability obligations and/or rights consistent with this + License. However, in accepting such obligations, You may act only + on Your own behalf and on Your sole responsibility, not on behalf + of any other Contributor, and only if You agree to indemnify, + defend, and hold each Contributor harmless for any liability + incurred by, or claims asserted against, such Contributor by reason + of your accepting any such warranty or additional liability. + + END OF TERMS AND CONDITIONS + + APPENDIX: How to apply the Apache License to your work. + + To apply the Apache License to your work, attach the following + boilerplate notice, with the fields enclosed by brackets "[]" + replaced with your own identifying information. (Don't include + the brackets!) The text should be enclosed in the appropriate + comment syntax for the file format. We also recommend that a + file or class name and description of purpose be included on the + same "printed page" as the copyright notice for easier + identification within third-party archives. + + Copyright [yyyy] [name of copyright owner] + + Licensed under the Apache License, Version 2.0 (the "License"); + you may not use this file except in compliance with the License. + You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + += LICENSE 75cd5500580317e758b5e984e017524dc961140e4889f7d427f85e41 = +================================================================================ + ================================================================================ = github.com/lestrrat-go/strftime licensed under: = diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index cb0d6fcff0..7994b241c2 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -41,6 +41,7 @@ import ( type diffOutput int type diffPart int +type diffMode int const ( SchemaOnlyDiff diffPart = 1 // 0b0001 @@ -62,6 +63,7 @@ const ( CachedFlag = "cached" SkinnyFlag = "skinny" MergeBase = "merge-base" + DiffMode = "diff-mode" ) var diffDocs = cli.CommandDocumentationContent{ @@ -87,6 +89,8 @@ Show changes between the working and staged tables, changes between the working The diffs displayed can be limited to show the first N by providing the parameter {{.EmphasisLeft}}--limit N{{.EmphasisRight}} where {{.EmphasisLeft}}N{{.EmphasisRight}} is the number of diffs to display. To filter which data rows are displayed, use {{.EmphasisLeft}}--where {{.EmphasisRight}}. Table column names in the filter expression must be prefixed with {{.EmphasisLeft}}from_{{.EmphasisRight}} or {{.EmphasisLeft}}to_{{.EmphasisRight}}, e.g. {{.EmphasisLeft}}to_COLUMN_NAME > 100{{.EmphasisRight}} or {{.EmphasisLeft}}from_COLUMN_NAME + to_COLUMN_NAME = 0{{.EmphasisRight}}. + +The {{.EmphasisLeft}}--diff-mode{{.EmphasisRight}} argument controls how modified rows are presented when the format output is set to {{.EmphasisLeft}}tabular{{.EmphasisRight}}. When set to {{.EmphasisLeft}}row{{.EmphasisRight}}, modified rows are presented as old and new rows. When set to {{.EmphasisLeft}}line{{.EmphasisRight}}, modified rows are presented as a single row, and changes are presented using "+" and "-" within the column. When set to {{.EmphasisLeft}}in-place{{.EmphasisRight}}, modified rows are presented as a single row, and changes are presented side-by-side with a color distinction (requires a color-enabled terminal). When set to {{.EmphasisLeft}}context{{.EmphasisRight}}, rows that contain at least one column that spans multiple lines uses {{.EmphasisLeft}}line{{.EmphasisRight}}, while all other rows use {{.EmphasisLeft}}row{{.EmphasisRight}}. The default value is {{.EmphasisLeft}}context{{.EmphasisRight}}. `, Synopsis: []string{ `[options] [{{.LessThan}}commit{{.GreaterThan}}] [{{.LessThan}}tables{{.GreaterThan}}...]`, @@ -97,6 +101,7 @@ To filter which data rows are displayed, use {{.EmphasisLeft}}--where 0 { query += " where " + dArgs.where @@ -782,7 +796,7 @@ func diffRows( } } - err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter, modifiedColNames, dArgs.skinny) + err = writeDiffResults(sqlCtx, sch, unionSch, rowIter, rowWriter, modifiedColNames, dArgs) if err != nil { return errhand.BuildDError("Error running diff query:\n%s", query).AddCause(err).Build() } @@ -827,7 +841,7 @@ func writeDiffResults( iter sql.RowIter, writer diff.SqlRowDiffWriter, modifiedColNames map[string]bool, - filterChangedCols bool, + dArgs *diffArgs, ) error { ds, err := newDiffSplitter(diffQuerySch, targetSch) if err != nil { @@ -847,7 +861,7 @@ func writeDiffResults( return err } - if filterChangedCols { + if dArgs.skinny { var filteredOldRow, filteredNewRow rowDiff for i, changeType := range newRow.colDiffs { if (changeType == diff.Added|diff.Removed) || modifiedColNames[targetSch[i].Name] { @@ -869,17 +883,21 @@ func writeDiffResults( newRow = filteredNewRow } - if oldRow.row != nil { - err := writer.WriteRow(ctx, oldRow.row, oldRow.rowDiff, oldRow.colDiffs) - if err != nil { + // We are guaranteed to have "ModeRow" for writers that do not support combined rows + if dArgs.diffMode != diff.ModeRow && oldRow.rowDiff == diff.ModifiedOld && newRow.rowDiff == diff.ModifiedNew { + if err = writer.WriteCombinedRow(ctx, oldRow.row, newRow.row, dArgs.diffMode); err != nil { return err } - } - - if newRow.row != nil { - err := writer.WriteRow(ctx, newRow.row, newRow.rowDiff, newRow.colDiffs) - if err != nil { - return err + } else { + if oldRow.row != nil { + if err = writer.WriteRow(ctx, oldRow.row, oldRow.rowDiff, oldRow.colDiffs); err != nil { + return err + } + } + if newRow.row != nil { + if err = writer.WriteRow(ctx, newRow.row, newRow.rowDiff, newRow.colDiffs); err != nil { + return err + } } } } diff --git a/go/go.mod b/go/go.mod index 4d549c6c14..9b223677ec 100644 --- a/go/go.mod +++ b/go/go.mod @@ -31,7 +31,7 @@ require ( github.com/pkg/errors v0.9.1 github.com/pkg/profile v1.5.0 github.com/rivo/uniseg v0.2.0 - github.com/sergi/go-diff v1.1.0 // indirect + github.com/sergi/go-diff v1.1.0 github.com/shopspring/decimal v1.2.0 github.com/silvasur/buzhash v0.0.0-20160816060738-9bdec3dec7c6 github.com/sirupsen/logrus v1.8.1 @@ -62,6 +62,7 @@ require ( github.com/google/flatbuffers v2.0.6+incompatible github.com/jmoiron/sqlx v1.3.4 github.com/kch42/buzhash v0.0.0-20160816060738-9bdec3dec7c6 + github.com/kylelemons/godebug v1.1.0 github.com/mitchellh/go-ps v1.0.0 github.com/prometheus/client_golang v1.13.0 github.com/rs/zerolog v1.28.0 diff --git a/go/go.sum b/go/go.sum index fa1a799b07..9935c03f3e 100644 --- a/go/go.sum +++ b/go/go.sum @@ -412,6 +412,8 @@ github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= +github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc= +github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw= github.com/lestrrat-go/envload v0.0.0-20180220234015-a3eb8ddeffcc h1:RKf14vYWi2ttpEmkA4aQ3j4u9dStX2t4M8UM6qqNsG8= github.com/lestrrat-go/envload v0.0.0-20180220234015-a3eb8ddeffcc/go.mod h1:kopuH9ugFRkIXf3YoqHKyrJ9YfUFsckUU9S7B+XP+is= github.com/lestrrat-go/strftime v1.0.4 h1:T1Rb9EPkAhgxKqbcMIPguPq8glqXTA1koF8n9BHElA8= diff --git a/go/libraries/doltcore/diff/diff.go b/go/libraries/doltcore/diff/diff.go index d00b9c49d1..4bbc1173f8 100755 --- a/go/libraries/doltcore/diff/diff.go +++ b/go/libraries/doltcore/diff/diff.go @@ -40,6 +40,16 @@ const ( ModifiedNew ) +// Mode is an enum that represents the presentation of a diff +type Mode int + +const ( + ModeRow Mode = 0 + ModeLine Mode = 1 + ModeInPlace Mode = 2 + ModeContext Mode = 3 +) + type RowDiffer interface { // Start starts the RowDiffer. Start(ctx context.Context, from, to types.Map) @@ -63,6 +73,9 @@ type SqlRowDiffWriter interface { // the input row. WriteRow(ctx context.Context, row sql.Row, diffType ChangeType, colDiffTypes []ChangeType) error + // WriteCombinedRow writes the diff of the rows given as a single, combined row. + WriteCombinedRow(ctx context.Context, oldRow, newRow sql.Row, mode Mode) error + // Close finalizes the work of this writer. Close(ctx context.Context) error } diff --git a/go/libraries/doltcore/table/typed/json/json_diff_writer.go b/go/libraries/doltcore/table/typed/json/json_diff_writer.go index ec2864a6cb..35ff7f4b11 100755 --- a/go/libraries/doltcore/table/typed/json/json_diff_writer.go +++ b/go/libraries/doltcore/table/typed/json/json_diff_writer.go @@ -127,6 +127,10 @@ func (j *JsonDiffWriter) WriteRow( return nil } +func (j *JsonDiffWriter) WriteCombinedRow(ctx context.Context, oldRow, newRow sql.Row, mode diff.Mode) error { + return fmt.Errorf("json format is unable to output diffs for combined rows") +} + func (j *JsonDiffWriter) Close(ctx context.Context) error { err := iohelp.WriteAll(j.wr, []byte("]")) if err != nil { diff --git a/go/libraries/doltcore/table/untyped/sqlexport/sql_diff_writer.go b/go/libraries/doltcore/table/untyped/sqlexport/sql_diff_writer.go index 1a99060305..09d351e945 100755 --- a/go/libraries/doltcore/table/untyped/sqlexport/sql_diff_writer.go +++ b/go/libraries/doltcore/table/untyped/sqlexport/sql_diff_writer.go @@ -39,6 +39,8 @@ type SqlDiffWriter struct { autocommitOff bool } +var _ diff.SqlRowDiffWriter = SqlDiffWriter{} + func NewSqlDiffWriter(tableName string, schema schema.Schema, wr io.WriteCloser) *SqlDiffWriter { return &SqlDiffWriter{ tableName: tableName, @@ -95,6 +97,10 @@ func (w SqlDiffWriter) WriteRow( } } +func (w SqlDiffWriter) WriteCombinedRow(ctx context.Context, oldRow, newRow sql.Row, mode diff.Mode) error { + return fmt.Errorf("sql format is unable to output diffs for combined rows") +} + func (w SqlDiffWriter) Close(ctx context.Context) error { return w.writeCloser.Close() } diff --git a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_conflict_tablewriter.go b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_conflict_tablewriter.go index f443c82bf9..034cf50055 100644 --- a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_conflict_tablewriter.go +++ b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_conflict_tablewriter.go @@ -71,7 +71,7 @@ func (w FixedWidthConflictTableWriter) WriteRow( } newRow := append(sql.Row{diffMarker, version}, row...) - return w.tableWriter.WriteColoredRow(ctx, newRow, rowColorsForDiffType(rowDiffType, 2, len(row))) + return w.tableWriter.WriteColoredSqlRow(ctx, newRow, rowColorsForDiffType(rowDiffType, 2, len(row))) } func (w FixedWidthConflictTableWriter) Close(ctx context.Context) error { diff --git a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_diff_tablewriter.go b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_diff_tablewriter.go index d963d0c623..98ec909524 100755 --- a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_diff_tablewriter.go +++ b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_diff_tablewriter.go @@ -18,10 +18,13 @@ import ( "context" "fmt" "io" + "strings" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/types" "github.com/fatih/color" + computeDiff "github.com/kylelemons/godebug/diff" + "github.com/sergi/go-diff/diffmatchpatch" "github.com/dolthub/dolt/go/libraries/doltcore/diff" ) @@ -32,6 +35,8 @@ type FixedWidthDiffTableWriter struct { tableWriter *FixedWidthTableWriter } +var _ diff.SqlRowDiffWriter = FixedWidthDiffTableWriter{} + func NewFixedWidthDiffTableWriter(schema sql.Schema, wr io.WriteCloser, numSamples int) *FixedWidthDiffTableWriter { // leading diff type column with empty name schema = append(sql.Schema{&sql.Column{ @@ -70,7 +75,109 @@ func (w FixedWidthDiffTableWriter) WriteRow( newRow := append(sql.Row{diffMarker}, row...) newColDiffTypes := append([]diff.ChangeType{rowDiffType}, colDiffTypes...) - return w.tableWriter.WriteColoredRow(ctx, newRow, colorsForDiffTypes(newColDiffTypes)) + return w.tableWriter.WriteColoredSqlRow(ctx, newRow, colorsForDiffTypes(newColDiffTypes)) +} + +func (w FixedWidthDiffTableWriter) WriteCombinedRow(ctx context.Context, oldRow, newRow sql.Row, mode diff.Mode) error { + combinedRow := make([]string, len(oldRow)+1) + oldRowStrs := make([]string, len(combinedRow)) + newRowStrs := make([]string, len(combinedRow)) + columnDiffs := make([]bool, len(combinedRow)) + widths := make([]FixedWidthString, len(combinedRow)) + hasNewlines := false + + combinedRow[0] = "*" + oldRowStrs[0] = "<" + newRowStrs[0] = ">" + columnDiffs[0] = true + widths[0] = NewFixedWidthString(combinedRow[0]) + + for i := range oldRow { + var err error + oldRowStrs[i+1], err = w.tableWriter.stringValue(i+1, oldRow[i]) + if err != nil { + return err + } + newRowStrs[i+1], err = w.tableWriter.stringValue(i+1, newRow[i]) + if err != nil { + return err + } + combinedRow[i+1], columnDiffs[i+1], widths[i+1] = w.generateTextDiff(oldRowStrs[i+1], newRowStrs[i+1], mode == diff.ModeInPlace) + hasNewlines = hasNewlines || (columnDiffs[i+1] && len(widths[i+1].Lines) > 2) || (!columnDiffs[i+1] && len(widths[i+1].Lines) > 1) + } + + if mode == diff.ModeContext && !hasNewlines { + oldRowColors := make([]*color.Color, len(combinedRow)) + newRowColors := make([]*color.Color, len(combinedRow)) + for i, hasDiff := range columnDiffs { + if hasDiff { + oldRowColors[i] = colorModifiedOld + newRowColors[i] = colorModifiedNew + } + } + err := w.tableWriter.WriteColoredRow(ctx, oldRowStrs, NewFixedWidthStrings(oldRowStrs), oldRowColors) + if err != nil { + return err + } + return w.tableWriter.WriteColoredRow(ctx, newRowStrs, NewFixedWidthStrings(newRowStrs), newRowColors) + } + return w.tableWriter.WriteColoredRow(ctx, combinedRow, widths, nil) +} + +// generateTextDiff returns a new string that represents a diff between the old and new string. The returned string will +// have color applied to it. +func (w FixedWidthDiffTableWriter) generateTextDiff(oldStr string, newStr string, inPlace bool) (result string, hasDiff bool, width FixedWidthString) { + // The diff routines will modify the strings, and we should just return the original if there will be no diff + if oldStr == newStr { + return oldStr, false, NewFixedWidthString(oldStr) + } + + // coloredStr will be the string that is displayed, which has had color applied to it + var coloredStr strings.Builder + // uncoloredStr is the string that is measured to determine display width, as the colors interfere with measuring + var uncoloredStr strings.Builder + if inPlace { + dmp := diffmatchpatch.New() + diffs := dmp.DiffMain(oldStr, newStr, false) + for _, diffPart := range diffs { + uncoloredStr.WriteString(diffPart.Text) + // We need to end color before any newlines, and reapply it after newlines, else the color will trail to the + // next line. + for i, part := range strings.Split(diffPart.Text, "\n") { + if i > 0 { + coloredStr.WriteRune('\n') + } + switch diffPart.Type { + case diffmatchpatch.DiffEqual: + coloredStr.WriteString(part) + case diffmatchpatch.DiffInsert: + coloredStr.WriteString(colorModifiedNew.Sprint(part)) + case diffmatchpatch.DiffDelete: + coloredStr.WriteString(colorModifiedOld.Sprint(part)) + } + } + } + } else { + diffStrs := strings.Split(computeDiff.Diff(oldStr, newStr), "\n") + for i, diffStr := range diffStrs { + if i > 0 { + uncoloredStr.WriteRune('\n') + coloredStr.WriteRune('\n') + } + if len(diffStr) > 0 { + uncoloredStr.WriteString(diffStr) + switch diffStr[0] { + case '+': + coloredStr.WriteString(colorModifiedNew.Sprint(diffStr)) + case '-': + coloredStr.WriteString(colorModifiedOld.Sprint(diffStr)) + default: + coloredStr.WriteString(diffStr) + } + } + } + } + return coloredStr.String(), true, ColoredStringWidth(coloredStr.String(), uncoloredStr.String()) } func colorsForDiffTypes(colDiffTypes []diff.ChangeType) []*color.Color { diff --git a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go index 58a687053e..b4c509db1c 100644 --- a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go +++ b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter.go @@ -58,7 +58,7 @@ var _ table.SqlRowWriter = (*FixedWidthTableWriter)(nil) type tableRow struct { columns []string colors []*color.Color - widths []DisplayString + widths []FixedWidthString height int } @@ -78,7 +78,7 @@ func NewFixedWidthTableWriter(schema sql.Schema, wr io.WriteCloser, numSamples i func (w *FixedWidthTableWriter) seedColumnWidthsWithColumnNames() { for i := range w.schema { colName := w.schema[i].Name - stringWidth := StringWidth(colName) + stringWidth := NewFixedWidthString(colName) w.printWidths[i] = stringWidth.TotalWidth } } @@ -104,19 +104,41 @@ func (w *FixedWidthTableWriter) Close(ctx context.Context) error { return w.closer.Close() } +var ( + colorAdded = color.New(color.Bold, color.FgGreen) + colorModifiedOld = color.New(color.FgRed) + colorModifiedNew = color.New(color.FgGreen) + colorRemoved = color.New(color.Bold, color.FgRed) +) + var colDiffColors = map[diff.ChangeType]*color.Color{ - diff.Added: color.New(color.Bold, color.FgGreen), - diff.ModifiedOld: color.New(color.FgRed), - diff.ModifiedNew: color.New(color.FgGreen), - diff.Removed: color.New(color.Bold, color.FgRed), + diff.Added: colorAdded, + diff.ModifiedOld: colorModifiedOld, + diff.ModifiedNew: colorModifiedNew, + diff.Removed: colorRemoved, } func (w *FixedWidthTableWriter) WriteSqlRow(ctx context.Context, r sql.Row) error { - return w.WriteColoredRow(ctx, r, nil) + return w.WriteColoredSqlRow(ctx, r, nil) } -func (w *FixedWidthTableWriter) WriteColoredRow(ctx context.Context, r sql.Row, colors []*color.Color) error { - if colors == nil { +// WriteColoredSqlRow writes the given SQL row to the buffer. If colors are nil, then uses the default color. +func (w *FixedWidthTableWriter) WriteColoredSqlRow(ctx context.Context, r sql.Row, colors []*color.Color) error { + strRow := make([]string, len(r)) + for i := range r { + str, err := w.stringValue(i, r[i]) + if err != nil { + return err + } + strRow[i] = str + } + return w.WriteColoredRow(ctx, strRow, nil, colors) +} + +// WriteColoredRow writes the given row to the buffer. If widths are nil, then calculates the widths on the given +// strings. If colors are nil, then uses the default color. +func (w *FixedWidthTableWriter) WriteColoredRow(ctx context.Context, r []string, widths []FixedWidthString, colors []*color.Color) error { + if len(colors) == 0 { colors = make([]*color.Color, len(r)) } @@ -131,7 +153,7 @@ func (w *FixedWidthTableWriter) WriteColoredRow(ctx context.Context, r sql.Row, // We immediately set to false as we're going to write more rows to the buffer w.flushedSampleBuffer = false } - strRow, err := w.sampleRow(r, colors) + strRow, err := w.sampleRow(r, widths, colors) if err != nil { return err } @@ -142,30 +164,32 @@ func (w *FixedWidthTableWriter) WriteColoredRow(ctx context.Context, r sql.Row, return nil } -func (w *FixedWidthTableWriter) sampleRow(r sql.Row, colors []*color.Color) (tableRow, error) { +func (w *FixedWidthTableWriter) sampleRow(r []string, widths []FixedWidthString, colors []*color.Color) (tableRow, error) { + if len(widths) == 0 { + widths = make([]FixedWidthString, len(r)) + } row := tableRow{ - columns: make([]string, len(r)), + columns: r, colors: colors, - widths: make([]DisplayString, len(r)), + widths: widths, height: 1, } - for i := range r { - str, err := w.stringValue(i, r[i]) - if err != nil { - return row, err + for i, str := range r { + var width FixedWidthString + if len(widths[i].Lines) == 0 { + width = NewFixedWidthString(str) + widths[i] = width + } else { + width = widths[i] } - stringWidth := StringWidth(str) - - if stringWidth.DisplayWidth > w.printWidths[i] { - w.printWidths[i] = stringWidth.DisplayWidth + if width.DisplayWidth > w.printWidths[i] { + w.printWidths[i] = width.DisplayWidth } - row.columns[i] = str - row.widths[i] = stringWidth - if len(stringWidth.Lines) > row.height { - row.height = len(stringWidth.Lines) + if len(width.Lines) > row.height { + row.height = len(width.Lines) } } @@ -241,7 +265,7 @@ func (w *FixedWidthTableWriter) rowToTableRow(row sql.Row, colors []*color.Color tRow := tableRow{ columns: make([]string, len(row)), colors: colors, - widths: make([]DisplayString, len(row)), + widths: make([]FixedWidthString, len(row)), height: 1, } @@ -252,7 +276,7 @@ func (w *FixedWidthTableWriter) rowToTableRow(row sql.Row, colors []*color.Color return tableRow{}, err } - stringWidth := StringWidth(tRow.columns[i]) + stringWidth := NewFixedWidthString(tRow.columns[i]) tRow.widths[i] = stringWidth if len(stringWidth.Lines) > tRow.height { tRow.height = len(stringWidth.Lines) @@ -277,7 +301,7 @@ func (w *FixedWidthTableWriter) writeHeader() error { colNameLine.WriteString("|") for i, name := range colNames { colNameLine.WriteString(" ") - width := StringWidth(name) + width := NewFixedWidthString(name) colNameLine.WriteString(fmt.Sprintf("%s%*s", name, w.printWidths[i]-width.TotalWidth, "")) colNameLine.WriteString(" |") } diff --git a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter_test.go b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter_test.go index 6565cd75bd..cf10642216 100755 --- a/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter_test.go +++ b/go/libraries/doltcore/table/untyped/tabular/fixedwidth_tablewriter_test.go @@ -156,4 +156,44 @@ func TestFixedWidthWriter(t *testing.T) { assert.Equal(t, expectedTableString, stringWr.String()) }) + + t.Run("Multiline string", func(t *testing.T) { + var stringWr StringBuilderCloser + tableWr := NewFixedWidthTableWriter(sch, &stringWr, 100) + + var expectedTableString = ` ++---------+------+-----------+ +| name | age | title | ++---------+------+-----------+ +| Michael | 43 | Regional | +| Scott | | Manager | +| Pam | 25 | Secretary | +| Beasley | | | +| Dwight | 29 | Assistant | +| Schrute | | to | +| | | the | +| | | Regional | +| | | Manager | +| Jim | NULL | NULL | +| Halpêrt | | | ++---------+------+-----------+ +` + // strip off the first newline, inserted for nice printing + expectedTableString = strings.Replace(expectedTableString, "\n", "", 1) + + for i := range ages { + name := strings.Replace(names[i].(string), " ", "\n", -1) + title := titles[i] + if title != nil { + title = strings.Replace(title.(string), " ", "\n", -1) + } + err := tableWr.WriteSqlRow(context.Background(), sql.Row{name, ages[i], title}) + assert.NoError(t, err) + } + + err := tableWr.Close(context.Background()) + assert.NoError(t, err) + + assert.Equal(t, expectedTableString, stringWr.String()) + }) } diff --git a/go/libraries/doltcore/table/untyped/tabular/stringwidth.go b/go/libraries/doltcore/table/untyped/tabular/stringwidth.go index e3349d1fbb..fbfe2d7c57 100644 --- a/go/libraries/doltcore/table/untyped/tabular/stringwidth.go +++ b/go/libraries/doltcore/table/untyped/tabular/stringwidth.go @@ -19,10 +19,10 @@ import ( "github.com/rivo/uniseg" ) -// DisplayString contains all of the information needed to properly display a multiline string in tabular mode. -type DisplayString struct { - TotalWidth int - DisplayWidth int +// FixedWidthString contains all of the information needed to properly display a multiline string in tabular mode. +type FixedWidthString struct { + TotalWidth int // The combined display width of every line + DisplayWidth int // The display width of the longest line Lines []DisplayLine } @@ -33,11 +33,11 @@ type DisplayLine struct { ByteEnd int // ByteEnd is the ending offset of the original string that this line represents (excludes newline). } -// StringWidth returns the number of horizontal cells needed to print the given text. It splits the text into its +// NewFixedWidthString returns the number of horizontal cells needed to print the given text. It splits the text into its // grapheme clusters, calculates each cluster's width, and adds them up to a total. -func StringWidth(text string) DisplayString { +func NewFixedWidthString(text string) FixedWidthString { // An empty string will still have a single line, it will just be empty. - displayString := DisplayString{ + displayString := FixedWidthString{ TotalWidth: 0, DisplayWidth: 0, Lines: []DisplayLine{{0, 0, 0}}, @@ -79,3 +79,25 @@ func StringWidth(text string) DisplayString { } return displayString } + +// NewFixedWidthStrings returns a FixedWidthString slice from the given string slice. +func NewFixedWidthStrings(text []string) []FixedWidthString { + widths := make([]FixedWidthString, len(text)) + for i := range text { + widths[i] = NewFixedWidthString(text[i]) + } + return widths +} + +// ColoredStringWidth calculates the FixedWidthString for text that has been colored. Text colors interfere with the +// display width calculation, as they're considered displayable characters. This works by calculating the relevant +// information from the colored and uncolored text variants to return the correct FixedWidthString. +func ColoredStringWidth(coloredText string, uncoloredText string) FixedWidthString { + colored := NewFixedWidthString(coloredText) + uncolored := NewFixedWidthString(uncoloredText) + for i := range colored.Lines { + uncolored.Lines[i].ByteStart = colored.Lines[i].ByteStart + uncolored.Lines[i].ByteEnd = colored.Lines[i].ByteEnd + } + return uncolored +} diff --git a/integration-tests/bats/diff.bats b/integration-tests/bats/diff.bats index 6b28fdf9db..a463b99d19 100644 --- a/integration-tests/bats/diff.bats +++ b/integration-tests/bats/diff.bats @@ -22,6 +22,176 @@ teardown() { teardown_common } +@test "diff: row, line, in-place, context diff modes" { + # We're not using the test table, so we might as well delete it + dolt sql < | modify1 | CREATE PROCEDURE modify1() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 2 |" ]] || false + [[ "$output" =~ "| | | AS RESULTING |" ]] || false + [[ "$output" =~ "| | | FROM DUAL; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| < | modify2 | CREATE PROCEDURE modify2() SELECT 42 |" ]] || false + [[ "$output" =~ "| > | modify2 | CREATE PROCEDURE modify2() SELECT 43 |" ]] || false + [[ "$output" =~ "| - | remove | CREATE PROCEDURE remove() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 8; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "+---+---------+--------------------------------------+" ]] || false + + # Look at the line-by-line diff + run dolt diff original --diff-mode=line + [ "$status" -eq 0 ] + # Verify that standard diffs are still working + [[ "$output" =~ "| | PK |" ]] || false + [[ "$output" =~ "+---+----+" ]] || false + [[ "$output" =~ "| - | 2 |" ]] || false + [[ "$output" =~ "| + | 4 |" ]] || false + # Check the overall stored procedure diff (excluding dates since they're variable) + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + [[ "$output" =~ "| | name | create_stmt |" ]] || false + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + [[ "$output" =~ "| + | adding | CREATE PROCEDURE adding() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 9; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| * | modify1 | CREATE PROCEDURE modify1() BEGIN |" ]] || false + [[ "$output" =~ "| | | -DECLARE a INT DEFAULT 1; |" ]] || false + [[ "$output" =~ "| | | -SELECT a |" ]] || false + [[ "$output" =~ "| | | - AS RESULT; |" ]] || false + [[ "$output" =~ "| | | +SELECT 2 |" ]] || false + [[ "$output" =~ "| | | + AS RESULTING |" ]] || false + [[ "$output" =~ "| | | + FROM DUAL; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| * | modify2 | -CREATE PROCEDURE modify2() SELECT 42 |" ]] || false + [[ "$output" =~ "| | | +CREATE PROCEDURE modify2() SELECT 43 |" ]] || false + [[ "$output" =~ "| - | remove | CREATE PROCEDURE remove() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 8; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + + # Look at the in-place diff + run dolt diff original --diff-mode=in-place + [ "$status" -eq 0 ] + # Verify that standard diffs are still working + [[ "$output" =~ "| | PK |" ]] || false + [[ "$output" =~ "+---+----+" ]] || false + [[ "$output" =~ "| - | 2 |" ]] || false + [[ "$output" =~ "| + | 4 |" ]] || false + # Check the overall stored procedure diff (excluding dates since they're variable) + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + [[ "$output" =~ "| | name | create_stmt |" ]] || false + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + [[ "$output" =~ "| + | adding | CREATE PROCEDURE adding() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 9; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| * | modify1 | CREATE PROCEDURE modify1() BEGIN |" ]] || false + [[ "$output" =~ "| | | DECLARE a INT DEFAULT 1; |" ]] || false + [[ "$output" =~ "| | | SELECT a2 |" ]] || false + [[ "$output" =~ "| | | AS RESULTING |" ]] || false + [[ "$output" =~ "| | | FROM DUAL; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| * | modify2 | CREATE PROCEDURE modify2() SELECT 423 |" ]] || false + [[ "$output" =~ "| - | remove | CREATE PROCEDURE remove() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 8; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "+---+---------+---------------------------------------+" ]] || false + + # Look at the context diff + run dolt diff original --diff-mode=context + [ "$status" -eq 0 ] + # Verify that standard diffs are still working + [[ "$output" =~ "| | PK |" ]] || false + [[ "$output" =~ "+---+----+" ]] || false + [[ "$output" =~ "| - | 2 |" ]] || false + [[ "$output" =~ "| + | 4 |" ]] || false + # Check the overall stored procedure diff (excluding dates since they're variable) + [[ "$output" =~ "+---+---------+--------------------------------------+" ]] || false + [[ "$output" =~ "| | name | create_stmt |" ]] || false + [[ "$output" =~ "+---+---------+--------------------------------------+" ]] || false + [[ "$output" =~ "| + | adding | CREATE PROCEDURE adding() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 9; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| * | modify1 | CREATE PROCEDURE modify1() BEGIN |" ]] || false + [[ "$output" =~ "| | | -DECLARE a INT DEFAULT 1; |" ]] || false + [[ "$output" =~ "| | | -SELECT a |" ]] || false + [[ "$output" =~ "| | | - AS RESULT; |" ]] || false + [[ "$output" =~ "| | | +SELECT 2 |" ]] || false + [[ "$output" =~ "| | | + AS RESULTING |" ]] || false + [[ "$output" =~ "| | | + FROM DUAL; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "| < | modify2 | CREATE PROCEDURE modify2() SELECT 42 |" ]] || false + [[ "$output" =~ "| > | modify2 | CREATE PROCEDURE modify2() SELECT 43 |" ]] || false + [[ "$output" =~ "| - | remove | CREATE PROCEDURE remove() BEGIN |" ]] || false + [[ "$output" =~ "| | | SELECT 8; |" ]] || false + [[ "$output" =~ "| | | END |" ]] || false + [[ "$output" =~ "+---+---------+--------------------------------------+" ]] || false + + # Ensure that the context diff is the default + run dolt diff original + [ "$status" -eq 0 ] + [[ "$output" =~ "| * | modify1 | CREATE PROCEDURE modify1() BEGIN |" ]] || false + [[ "$output" =~ "| | | -SELECT a |" ]] || false + [[ "$output" =~ "| | | +SELECT 2 |" ]] || false + [[ "$output" =~ "| < | modify2 | CREATE PROCEDURE modify2() SELECT 42 |" ]] || false + [[ "$output" =~ "| > | modify2 | CREATE PROCEDURE modify2() SELECT 43 |" ]] || false +} + @test "diff: clean working set" { dolt add . dolt commit -m table @@ -152,7 +322,7 @@ teardown() { [[ ! "$output" =~ "- | 1" ]] || false [[ "$output" =~ "+ | 2" ]] || false - run dolt diff --merge-base branch1 + run dolt diff --merge-base branch1 [ "$status" -eq 0 ] [[ ! "$output" =~ "- | 1" ]] || false [[ "$output" =~ "+ | 2" ]] || false