From 040f1faeb9681fd86240deb7a62f9b248040f3ba Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Thu, 27 Oct 2022 10:26:55 -0700 Subject: [PATCH 1/2] Support Apple Diff timestamps Apple's diff (at least the one shipping with macOS Venture) does not include the fractional seconds or Timezone in it's output. This adds support for that so diffutils does not need to be installed. I first encountered this problem using golangci-lint on macOS after upgrading to macOS Ventura. --- diff/diff.go | 17 +++++++- diff/diff_test.go | 1 + diff/parse.go | 43 ++++++++++++------- diff/print.go | 12 ++++-- .../sample_multi_file_single_apple.diff | 29 +++++++++++++ 5 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 diff/testdata/sample_multi_file_single_apple.diff diff --git a/diff/diff.go b/diff/diff.go index 0f465b9..471abda 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -15,15 +15,19 @@ type FileDiff struct { // the original name of the file OrigName string // the original timestamp (nil if not present) - OrigTime *time.Time + OrigTime *time.Time + OrigTimeHasTZ bool // the new name of the file (often same as OrigName) NewName string // the new timestamp (nil if not present) - NewTime *time.Time + NewTime *time.Time + NewTimeHasTZ bool // extended header lines (e.g., git's "new mode ", "rename from ", etc.) Extended []string // hunks that were changed from orig to new Hunks []*Hunk + + includeTZ bool } // A Hunk represents a series of changes (additions or deletions) in a file's @@ -120,11 +124,20 @@ const onlyInMessage = "Only in %s: %s\n" // See https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html. const diffTimeParseLayout = "2006-01-02 15:04:05 -0700" +// Apple's diff is based on freebsd diff, which uses a timestamp format that does +// not include the timezone offset. +const diffTimeParseWithoutTZLayout = "2006-01-02 15:04:05" + // diffTimeFormatLayout is the layout used to format (i.e., print) the time in unified diff file // header timestamps. // See https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html. const diffTimeFormatLayout = "2006-01-02 15:04:05.000000000 -0700" +// diffTimeFormatWithoutTZLayout is the layout used to format (i.e., print) the time in unified diff file +// header timestamps without the timezone offset and the fractional seconds. This is used when the diff +// does not include the timezone offset and fractional seconds. +const diffTimeFormatWithoutTZLayout = "2006-01-02 15:04:05" + func (s *Stat) add(o Stat) { s.Added += o.Added s.Changed += o.Changed diff --git a/diff/diff_test.go b/diff/diff_test.go index 11c7b01..254c2eb 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -856,6 +856,7 @@ func TestParseMultiFileDiffAndPrintMultiFileDiff(t *testing.T) { }{ {filename: "sample_multi_file.diff", wantFileDiffs: 2}, {filename: "sample_multi_file_single.diff", wantFileDiffs: 1}, + {filename: "sample_multi_file_single_apple.diff", wantFileDiffs: 1}, {filename: "sample_multi_file_new.diff", wantFileDiffs: 3}, {filename: "sample_multi_file_deleted.diff", wantFileDiffs: 3}, {filename: "sample_multi_file_rename.diff", wantFileDiffs: 3}, diff --git a/diff/parse.go b/diff/parse.go index 4a0b823..71dfc45 100644 --- a/diff/parse.go +++ b/diff/parse.go @@ -217,15 +217,18 @@ func (r *FileDiffReader) ReadAllHeaders() (*FileDiff, error) { } var origTime, newTime *time.Time - fd.OrigName, fd.NewName, origTime, newTime, err = r.ReadFileHeaders() + var origTZ, newTZ bool + fd.OrigName, fd.NewName, origTime, newTime, origTZ, newTZ, err = r.ReadFileHeaders() if err != nil { return nil, err } if origTime != nil { fd.OrigTime = origTime + fd.OrigTimeHasTZ = origTZ } if newTime != nil { fd.NewTime = newTime + fd.NewTimeHasTZ = newTZ } return fd, nil @@ -247,22 +250,21 @@ func (r *FileDiffReader) HunksReader() *HunksReader { // start with "---" and "+++" with the orig/new file names and // timestamps). Or which starts with "Only in " with dir path and filename. // "Only in" message is supported in POSIX locale: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10 -func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimestamp, newTimestamp *time.Time, err error) { +func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimestamp, newTimestamp *time.Time, origTZ, newTZ bool, err error) { if r.fileHeaderLine != nil { if isOnlyMessage, source, filename := parseOnlyInMessage(r.fileHeaderLine); isOnlyMessage { return filepath.Join(string(source), string(filename)), - "", nil, nil, nil + "", nil, nil, false, false, nil } } - - origName, origTimestamp, err = r.readOneFileHeader([]byte("--- ")) + origName, origTimestamp, origTZ, err = r.readOneFileHeader([]byte("--- ")) if err != nil { - return "", "", nil, nil, err + return "", "", nil, nil, false, false, err } - newName, newTimestamp, err = r.readOneFileHeader([]byte("+++ ")) + newName, newTimestamp, newTZ, err = r.readOneFileHeader([]byte("+++ ")) if err != nil { - return "", "", nil, nil, err + return "", "", nil, nil, false, false, err } unquotedOrigName, err := strconv.Unquote(origName) @@ -274,21 +276,21 @@ func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimest newName = unquotedNewName } - return origName, newName, origTimestamp, newTimestamp, nil + return origName, newName, origTimestamp, newTimestamp, origTZ, newTZ, nil } // readOneFileHeader reads one of the file headers (prefix should be // either "+++ " or "--- "). -func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, timestamp *time.Time, err error) { +func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, timestamp *time.Time, hadTZ bool, err error) { var line []byte if r.fileHeaderLine == nil { var err error line, err = r.reader.readLine() if err == io.EOF { - return "", nil, &ParseError{r.line, r.offset, ErrNoFileHeader} + return "", nil, false, &ParseError{r.line, r.offset, ErrNoFileHeader} } else if err != nil { - return "", nil, err + return "", nil, false, err } } else { line = r.fileHeaderLine @@ -296,7 +298,7 @@ func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, time } if !bytes.HasPrefix(line, prefix) { - return "", nil, &ParseError{r.line, r.offset, ErrBadFileHeader} + return "", nil, false, &ParseError{r.line, r.offset, ErrBadFileHeader} } r.offset += int64(len(line)) @@ -306,16 +308,25 @@ func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, time trimmedLine := strings.TrimSpace(string(line)) // filenames that contain spaces may be terminated by a tab parts := strings.SplitN(trimmedLine, "\t", 2) filename = parts[0] + hadTZ = true if len(parts) == 2 { + var ts time.Time // Timestamp is optional, but this header has it. - ts, err := time.Parse(diffTimeParseLayout, parts[1]) + ts, err = time.Parse(diffTimeParseLayout, parts[1]) if err != nil { - return "", nil, err + var err1 error + ts, err1 = time.Parse(diffTimeParseWithoutTZLayout, parts[1]) + if err1 != nil { + return "", nil, hadTZ, err + } + hadTZ = false + ts = ts.In(time.Now().Location()) + err = nil } timestamp = &ts } - return filename, timestamp, err + return filename, timestamp, hadTZ, err } // OverflowError is returned when we have overflowed into the start diff --git a/diff/print.go b/diff/print.go index 012651a..ae15e56 100644 --- a/diff/print.go +++ b/diff/print.go @@ -49,10 +49,10 @@ func PrintFileDiff(d *FileDiff) ([]byte, error) { return buf.Bytes(), nil } - if err := printFileHeader(&buf, "--- ", d.OrigName, d.OrigTime); err != nil { + if err := printFileHeader(&buf, "--- ", d.OrigName, d.OrigTime, d.OrigTimeHasTZ); err != nil { return nil, err } - if err := printFileHeader(&buf, "+++ ", d.NewName, d.NewTime); err != nil { + if err := printFileHeader(&buf, "+++ ", d.NewName, d.NewTime, d.NewTimeHasTZ); err != nil { return nil, err } @@ -67,12 +67,16 @@ func PrintFileDiff(d *FileDiff) ([]byte, error) { return buf.Bytes(), nil } -func printFileHeader(w io.Writer, prefix string, filename string, timestamp *time.Time) error { +func printFileHeader(w io.Writer, prefix string, filename string, timestamp *time.Time, tz bool) error { if _, err := fmt.Fprint(w, prefix, filename); err != nil { return err } + format := diffTimeFormatLayout + if !tz { + format = diffTimeFormatWithoutTZLayout + } if timestamp != nil { - if _, err := fmt.Fprint(w, "\t", timestamp.Format(diffTimeFormatLayout)); err != nil { + if _, err := fmt.Fprint(w, "\t", timestamp.Format(format)); err != nil { return err } } diff --git a/diff/testdata/sample_multi_file_single_apple.diff b/diff/testdata/sample_multi_file_single_apple.diff new file mode 100644 index 0000000..9100c9d --- /dev/null +++ b/diff/testdata/sample_multi_file_single_apple.diff @@ -0,0 +1,29 @@ +diff -u a/oldname1 b/newname1 +--- oldname1 2009-10-11 15:12:20 ++++ newname1 2009-10-11 15:12:30 +@@ -1,3 +1,9 @@ ++This is an important ++notice! It should ++therefore be located at ++the beginning of this ++document! ++ + This part of the + document has stayed the + same from version to +@@ -5,16 +11,10 @@ + be shown if it doesn't + change. Otherwise, that + would not be helping to +-compress the size of the +-changes. +- +-This paragraph contains +-text that is outdated. +-It will be deleted in the +-near future. ++compress anything. + + It is important to spell +-check this dokument. On ++check this document. On From e0afa0dd20ebebd63c4b22abbc6bf00f4f6f766a Mon Sep 17 00:00:00 2001 From: Edward Muller Date: Fri, 28 Oct 2022 11:27:41 -0700 Subject: [PATCH 2/2] Keep data in UTC & preserve public API This addresses PR feedback. --- diff/diff.go | 13 ++----- diff/diff_test.go | 15 +++++--- diff/parse.go | 34 ++++++++----------- diff/print.go | 12 +++---- ...=> sample_multi_file_single_apple_in.diff} | 0 .../sample_multi_file_single_apple_out.diff | 29 ++++++++++++++++ 6 files changed, 60 insertions(+), 43 deletions(-) rename diff/testdata/{sample_multi_file_single_apple.diff => sample_multi_file_single_apple_in.diff} (100%) create mode 100644 diff/testdata/sample_multi_file_single_apple_out.diff diff --git a/diff/diff.go b/diff/diff.go index 471abda..81aa655 100644 --- a/diff/diff.go +++ b/diff/diff.go @@ -15,19 +15,15 @@ type FileDiff struct { // the original name of the file OrigName string // the original timestamp (nil if not present) - OrigTime *time.Time - OrigTimeHasTZ bool + OrigTime *time.Time // the new name of the file (often same as OrigName) NewName string // the new timestamp (nil if not present) - NewTime *time.Time - NewTimeHasTZ bool + NewTime *time.Time // extended header lines (e.g., git's "new mode ", "rename from ", etc.) Extended []string // hunks that were changed from orig to new Hunks []*Hunk - - includeTZ bool } // A Hunk represents a series of changes (additions or deletions) in a file's @@ -133,11 +129,6 @@ const diffTimeParseWithoutTZLayout = "2006-01-02 15:04:05" // See https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Unified.html. const diffTimeFormatLayout = "2006-01-02 15:04:05.000000000 -0700" -// diffTimeFormatWithoutTZLayout is the layout used to format (i.e., print) the time in unified diff file -// header timestamps without the timezone offset and the fractional seconds. This is used when the diff -// does not include the timezone offset and fractional seconds. -const diffTimeFormatWithoutTZLayout = "2006-01-02 15:04:05" - func (s *Stat) add(o Stat) { s.Added += o.Added s.Changed += o.Changed diff --git a/diff/diff_test.go b/diff/diff_test.go index 254c2eb..aaaa261 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -850,13 +850,14 @@ func TestParseFileDiffAndPrintFileDiff(t *testing.T) { func TestParseMultiFileDiffAndPrintMultiFileDiff(t *testing.T) { tests := []struct { - filename string - wantParseErr error - wantFileDiffs int // How many instances of diff.FileDiff are expected. + filename string + wantParseErr error + wantFileDiffs int // How many instances of diff.FileDiff are expected. + wantOutFileName string // If non-empty, the name of the file containing the expected output. }{ {filename: "sample_multi_file.diff", wantFileDiffs: 2}, {filename: "sample_multi_file_single.diff", wantFileDiffs: 1}, - {filename: "sample_multi_file_single_apple.diff", wantFileDiffs: 1}, + {filename: "sample_multi_file_single_apple_in.diff", wantFileDiffs: 1, wantOutFileName: "sample_multi_file_single_apple_out.diff"}, {filename: "sample_multi_file_new.diff", wantFileDiffs: 3}, {filename: "sample_multi_file_deleted.diff", wantFileDiffs: 3}, {filename: "sample_multi_file_rename.diff", wantFileDiffs: 3}, @@ -893,6 +894,12 @@ func TestParseMultiFileDiffAndPrintMultiFileDiff(t *testing.T) { if err != nil { t.Errorf("%s: PrintMultiFileDiff: %s", test.filename, err) } + if test.wantOutFileName != "" { + diffData, err = ioutil.ReadFile(filepath.Join("testdata", test.wantOutFileName)) + if err != nil { + t.Fatal(err) + } + } if !bytes.Equal(printed, diffData) { t.Errorf("%s: printed multi-file diff != original multi-file diff\n\n# PrintMultiFileDiff output - Original:\n%s", test.filename, cmp.Diff(diffData, printed)) } diff --git a/diff/parse.go b/diff/parse.go index 71dfc45..f45c2f2 100644 --- a/diff/parse.go +++ b/diff/parse.go @@ -217,18 +217,15 @@ func (r *FileDiffReader) ReadAllHeaders() (*FileDiff, error) { } var origTime, newTime *time.Time - var origTZ, newTZ bool - fd.OrigName, fd.NewName, origTime, newTime, origTZ, newTZ, err = r.ReadFileHeaders() + fd.OrigName, fd.NewName, origTime, newTime, err = r.ReadFileHeaders() if err != nil { return nil, err } if origTime != nil { fd.OrigTime = origTime - fd.OrigTimeHasTZ = origTZ } if newTime != nil { fd.NewTime = newTime - fd.NewTimeHasTZ = newTZ } return fd, nil @@ -250,21 +247,21 @@ func (r *FileDiffReader) HunksReader() *HunksReader { // start with "---" and "+++" with the orig/new file names and // timestamps). Or which starts with "Only in " with dir path and filename. // "Only in" message is supported in POSIX locale: https://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10 -func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimestamp, newTimestamp *time.Time, origTZ, newTZ bool, err error) { +func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimestamp, newTimestamp *time.Time, err error) { if r.fileHeaderLine != nil { if isOnlyMessage, source, filename := parseOnlyInMessage(r.fileHeaderLine); isOnlyMessage { return filepath.Join(string(source), string(filename)), - "", nil, nil, false, false, nil + "", nil, nil, nil } } - origName, origTimestamp, origTZ, err = r.readOneFileHeader([]byte("--- ")) + origName, origTimestamp, err = r.readOneFileHeader([]byte("--- ")) if err != nil { - return "", "", nil, nil, false, false, err + return "", "", nil, nil, err } - newName, newTimestamp, newTZ, err = r.readOneFileHeader([]byte("+++ ")) + newName, newTimestamp, err = r.readOneFileHeader([]byte("+++ ")) if err != nil { - return "", "", nil, nil, false, false, err + return "", "", nil, nil, err } unquotedOrigName, err := strconv.Unquote(origName) @@ -276,21 +273,21 @@ func (r *FileDiffReader) ReadFileHeaders() (origName, newName string, origTimest newName = unquotedNewName } - return origName, newName, origTimestamp, newTimestamp, origTZ, newTZ, nil + return origName, newName, origTimestamp, newTimestamp, nil } // readOneFileHeader reads one of the file headers (prefix should be // either "+++ " or "--- "). -func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, timestamp *time.Time, hadTZ bool, err error) { +func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, timestamp *time.Time, err error) { var line []byte if r.fileHeaderLine == nil { var err error line, err = r.reader.readLine() if err == io.EOF { - return "", nil, false, &ParseError{r.line, r.offset, ErrNoFileHeader} + return "", nil, &ParseError{r.line, r.offset, ErrNoFileHeader} } else if err != nil { - return "", nil, false, err + return "", nil, err } } else { line = r.fileHeaderLine @@ -298,7 +295,7 @@ func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, time } if !bytes.HasPrefix(line, prefix) { - return "", nil, false, &ParseError{r.line, r.offset, ErrBadFileHeader} + return "", nil, &ParseError{r.line, r.offset, ErrBadFileHeader} } r.offset += int64(len(line)) @@ -308,7 +305,6 @@ func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, time trimmedLine := strings.TrimSpace(string(line)) // filenames that contain spaces may be terminated by a tab parts := strings.SplitN(trimmedLine, "\t", 2) filename = parts[0] - hadTZ = true if len(parts) == 2 { var ts time.Time // Timestamp is optional, but this header has it. @@ -317,16 +313,14 @@ func (r *FileDiffReader) readOneFileHeader(prefix []byte) (filename string, time var err1 error ts, err1 = time.Parse(diffTimeParseWithoutTZLayout, parts[1]) if err1 != nil { - return "", nil, hadTZ, err + return "", nil, err } - hadTZ = false - ts = ts.In(time.Now().Location()) err = nil } timestamp = &ts } - return filename, timestamp, hadTZ, err + return filename, timestamp, err } // OverflowError is returned when we have overflowed into the start diff --git a/diff/print.go b/diff/print.go index ae15e56..012651a 100644 --- a/diff/print.go +++ b/diff/print.go @@ -49,10 +49,10 @@ func PrintFileDiff(d *FileDiff) ([]byte, error) { return buf.Bytes(), nil } - if err := printFileHeader(&buf, "--- ", d.OrigName, d.OrigTime, d.OrigTimeHasTZ); err != nil { + if err := printFileHeader(&buf, "--- ", d.OrigName, d.OrigTime); err != nil { return nil, err } - if err := printFileHeader(&buf, "+++ ", d.NewName, d.NewTime, d.NewTimeHasTZ); err != nil { + if err := printFileHeader(&buf, "+++ ", d.NewName, d.NewTime); err != nil { return nil, err } @@ -67,16 +67,12 @@ func PrintFileDiff(d *FileDiff) ([]byte, error) { return buf.Bytes(), nil } -func printFileHeader(w io.Writer, prefix string, filename string, timestamp *time.Time, tz bool) error { +func printFileHeader(w io.Writer, prefix string, filename string, timestamp *time.Time) error { if _, err := fmt.Fprint(w, prefix, filename); err != nil { return err } - format := diffTimeFormatLayout - if !tz { - format = diffTimeFormatWithoutTZLayout - } if timestamp != nil { - if _, err := fmt.Fprint(w, "\t", timestamp.Format(format)); err != nil { + if _, err := fmt.Fprint(w, "\t", timestamp.Format(diffTimeFormatLayout)); err != nil { return err } } diff --git a/diff/testdata/sample_multi_file_single_apple.diff b/diff/testdata/sample_multi_file_single_apple_in.diff similarity index 100% rename from diff/testdata/sample_multi_file_single_apple.diff rename to diff/testdata/sample_multi_file_single_apple_in.diff diff --git a/diff/testdata/sample_multi_file_single_apple_out.diff b/diff/testdata/sample_multi_file_single_apple_out.diff new file mode 100644 index 0000000..3bcad6c --- /dev/null +++ b/diff/testdata/sample_multi_file_single_apple_out.diff @@ -0,0 +1,29 @@ +diff -u a/oldname1 b/newname1 +--- oldname1 2009-10-11 15:12:20.000000000 +0000 ++++ newname1 2009-10-11 15:12:30.000000000 +0000 +@@ -1,3 +1,9 @@ ++This is an important ++notice! It should ++therefore be located at ++the beginning of this ++document! ++ + This part of the + document has stayed the + same from version to +@@ -5,16 +11,10 @@ + be shown if it doesn't + change. Otherwise, that + would not be helping to +-compress the size of the +-changes. +- +-This paragraph contains +-text that is outdated. +-It will be deleted in the +-near future. ++compress anything. + + It is important to spell +-check this dokument. On ++check this document. On