Skip to content

Stop treating stuff like vertical tabs as line breaks when dealing with unified diffs#435

Merged
ExplodingCabbage merged 5 commits into6.0.0-stagingfrom
rip-out-handling-for-vertical-tabs-and-stuff
Dec 27, 2023
Merged

Stop treating stuff like vertical tabs as line breaks when dealing with unified diffs#435
ExplodingCabbage merged 5 commits into6.0.0-stagingfrom
rip-out-handling-for-vertical-tabs-and-stuff

Conversation

@ExplodingCabbage
Copy link
Collaborator

@ExplodingCabbage ExplodingCabbage commented Dec 20, 2023 •
edited
Loading

Tools like the Unix diff and patch CLI tools treat \v, \f, etc as just ordinary characters, but jsdiff currently treats them as line breaks. That creates an unfortunate incompatibility: if you generate a unified diff format diff with diff -u foo bar, and some of the lines in files foo and bar contained a \v or whatever, then jsdiff will parse the patch wrongly.

To see the problem, checkout the first commit on this PR (where I've added the test but not yet fixed jsdiff's behaviour to make the test pass) and run yarn test. You'll get this failure:

      1) should treat vertical tabs like ordinary characters


  179 passing (137ms)
  1 failing

  1) patch/parse
       #parse
         should treat vertical tabs like ordinary characters:

      AssertionError: expected [ Array(1) ] to deeply equal [ Array(1) ]
      + expected - actual

           "hunks": [
             {
               "linedelimiters": [
                 "\n"
      -          "\u000b"
      +          "\n"
      +          "\n"
      +          "\n"
      +          "\n"
      +          "\n"
               ]
               "lines": [
                 " foo"
      -          "-bar"
      +          "-bar\u000bbar"
      +          "+barry\u000bbarry"
      +          " baz"
      +          " qux"
      +          "\\ No newline at end of file"
               ]
               "newLines": 4
               "newStart": 1
               "oldLines": 4
      
      at Context.<anonymous> (test/patch/parse.js:647:19)
      at process.processImmediate (node:internal/timers:478:21)

Notice that it's not just the line delimiters that are wrong, but that the actual array of lines in the hunk is too short as a result of the parser choking on the \v character - all the lines after the \v have simply disappeared from our hunk!

@ExplodingCabbage ExplodingCabbage force-pushed the rip-out-handling-for-vertical-tabs-and-stuff branch from 44d0170 to 51c1645 Compare December 20, 2023 16:24
@ExplodingCabbage ExplodingCabbage force-pushed the rip-out-handling-for-vertical-tabs-and-stuff branch from 51c1645 to 0e5bed8 Compare December 20, 2023 16:25
@ExplodingCabbage ExplodingCabbage marked this pull request as ready for review December 20, 2023 16:28
@ExplodingCabbage ExplodingCabbage mentioned this pull request Dec 20, 2023
Closed
@ExplodingCabbage ExplodingCabbage changed the base branch from master to 6.0.0-staging December 27, 2023 12:24
@ExplodingCabbage ExplodingCabbage merged commit a4eac49 into 6.0.0-staging Dec 27, 2023
@ExplodingCabbage ExplodingCabbage deleted the rip-out-handling-for-vertical-tabs-and-stuff branch December 27, 2023 13:07
@kibanamachine kibanamachine mentioned this pull request Jan 6, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

breaking-change bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ExplodingCabbage