Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged

Conversation

ExplodingCabbage
Copy link
Collaborator

@ExplodingCabbage ExplodingCabbage commented Dec 20, 2023

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 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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant