Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Hotfix in restore discard history of files with same name #2676

Merged
merged 2 commits into from
May 9, 2021

Conversation

Robert-N7
Copy link
Contributor

Description of the Change

This hotfix restores the discard history of two files with the same name correctly by including the directory of the file in the result path.

Screenshot or Gif

N/A

Applicable Issues

Fixes #2172

@codecov
Copy link

codecov bot commented May 8, 2021

Codecov Report

Merging #2676 (176fdc7) into master (853ea19) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2676      +/-   ##
==========================================
- Coverage   93.46%   93.46%   -0.01%     
==========================================
  Files         237      237              
  Lines       13213    13213              
  Branches     1900     1900              
==========================================
- Hits        12350    12349       -1     
- Misses        863      864       +1     
Impacted Files Coverage Δ
lib/models/discard-history.js 99.00% <100.00%> (ø)
lib/atom/gutter.js 90.47% <0.00%> (-2.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 853ea19...176fdc7. Read the comment docs.

@smashwilson
Copy link
Contributor

The test failures seem to be unrelated so I'm not particularly concerned about those. But I think this isn't sufficient yet - I traced through and found a place where resultPath is passed to fs.writeFile, in such a way that I believe would break if its parent directory does not exist:

await fs.writeFile(resolvedResultPath, output, {encoding: 'utf8'});

I believe we'll also need to add calls to fs.mkdirs before each place the computed resultPath is used, now that it could be a subdirectory. I'm also not certain if fs.copy will create intermediate directories on the path to its destination, either.

Unfortunately DiscardHistory is lacking in our test coverage so it's hard to guarantee that we'll have caught them all. If you're really feeling motivated I would love to see a test suite started for this class 😇

@Robert-N7
Copy link
Contributor Author

The directory should exist, as it is created a few lines before the resultPath is declared.

await mkdirp(dir);

I will update the code to reflect by joining with the dir variable. I may look into creating tests at another time.

@smashwilson
Copy link
Contributor

Oh! Right you are. Works for me 👍🏻

@smashwilson smashwilson merged commit 9e83fb4 into atom:master May 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Undo Last Discard" restores files with same name with content of one of the files
2 participants