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

Fixes #13 Rename using diffs and parse diffs using diff-parse/diff npm modules #477

Merged
merged 14 commits into from
Sep 30, 2016

Conversation

ramya-rao-a
Copy link
Contributor

@ramya-rao-a ramya-rao-a commented Sep 14, 2016

Changes:

  • Update goRename.ts to use gorename -d that would give diffs instead of writing the files on disk with the refactored code
  • The formatting tools gofmt, go imports and goreturns have an option to return diffs instead of the whole formatted content. Therefore update goFormat.ts to use the -d option as well.
  • When diff tool does not exist on the machine, gorename/gofmt/goimports/goreturns -d will fail. Therefore, in these cases, revert to existing functionality which is write to disk files in case of rename and calculate diffs on our own in case of formatting
  • Node modules for generating and parsing diffs
    • diff-match-patch has a way to parse diff content only if the diff is for single file. Since rename can affect multiple files, this module cannot be used by goRename.ts
    • diff module can generate as well as parse diffs. Therefore diff-match-patch has been replaced by diff module

@ramya-rao-a ramya-rao-a changed the title Fixes #13 Rename using diffs and parse diffs using diff-parse Fixes #13 Rename using diffs and parse diffs using diff-parse/diff npm modules Sep 15, 2016
Copy link
Contributor

@lukehoban lukehoban left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good.

A bit of a shame we have to keep both codepaths here, but can't think of a better alternative.

I wasn't able to carefully review all the new diff parsing logic, but in testing in several projects I didn't run into any issues.

@ramya-rao-a ramya-rao-a merged commit a0fa2d2 into microsoft:master Sep 30, 2016
@ramya-rao-a ramya-rao-a deleted the rename-via-diffs branch September 30, 2016 17:14
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.

3 participants