-
Notifications
You must be signed in to change notification settings - Fork 501
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
Allow patchs in diff format #83
Changes from all commits
0c9dd6d
d501881
c238f85
54edda7
836a2f8
720f5ae
0c2af7b
d723875
c6b6a02
3908658
09b7efe
f91668b
3958558
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,9 +23,9 @@ | |
}, | ||
"main": "./lib", | ||
"scripts": { | ||
"test": "grunt" | ||
"prepublish": "grunt build", | ||
"test": "grunt test" | ||
}, | ||
"dependencies": {}, | ||
"devDependencies": { | ||
"async": "^1.4.2", | ||
"babel": "^5.8.23", | ||
|
@@ -60,6 +60,5 @@ | |
"semver": "^5.0.3", | ||
"webpack": "^1.12.2", | ||
"webpack-dev-server": "^1.12.0" | ||
}, | ||
"optionalDependencies": {} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please keep the changes focused on this PR. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,19 +20,21 @@ export function applyPatch(source, uniDiff, options = {}) { | |
compareLine = options.compareLine || ((lineNumber, line, operation, patchContent) => line === patchContent), | ||
errorCount = 0, | ||
fuzzFactor = options.fuzzFactor || 0, | ||
minLine = 0, | ||
offset = 0, | ||
|
||
removeEOFNL, | ||
addEOFNL; | ||
|
||
for (let i = 0; i < hunks.length; i++) { | ||
let hunk = hunks[i], | ||
toPos = hunk.newStart - 1; | ||
|
||
// Sanity check the input string. Bail if we don't match. | ||
/** | ||
* Checks if the hunk exactly fits on the provided location | ||
*/ | ||
function hunkFits(hunk, toPos) { | ||
for (let j = 0; j < hunk.lines.length; j++) { | ||
let line = hunk.lines[j], | ||
operation = line[0], | ||
content = line.substr(1); | ||
|
||
if (operation === ' ' || operation === '-') { | ||
// Context sanity check | ||
if (!compareLine(toPos + 1, lines[toPos], operation, content)) { | ||
|
@@ -42,7 +44,90 @@ export function applyPatch(source, uniDiff, options = {}) { | |
return false; | ||
} | ||
} | ||
toPos++; | ||
} | ||
} | ||
|
||
return true; | ||
} | ||
|
||
function distanceIterator(toPos, minLine, maxLine) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should probably be a separate module to help make things clearer. |
||
let wantForward = true, | ||
backwardExhausted = false, | ||
forwardExhausted = false, | ||
localOffset = 1; | ||
|
||
return function iterator() { | ||
if (wantForward && !forwardExhausted) { | ||
if (backwardExhausted) { | ||
localOffset++; | ||
} else { | ||
wantForward = false; | ||
} | ||
|
||
// Check if trying to fit beyond text length, and if not, check it fits | ||
// after offset location (or desired location on first iteration) | ||
if (toPos + localOffset <= maxLine) { | ||
return localOffset; | ||
} | ||
|
||
forwardExhausted = true; | ||
} | ||
|
||
if (!backwardExhausted) { | ||
if (!forwardExhausted) { | ||
wantForward = true; | ||
} | ||
|
||
// Check if trying to fit before text beginning, and if not, check it fits | ||
// before offset location | ||
if (minLine <= toPos - localOffset) { | ||
return -localOffset++; | ||
} | ||
|
||
backwardExhausted = true; | ||
return iterator(); | ||
} | ||
|
||
// We tried to fit hunk before text beginning and beyond text lenght, then | ||
// hunk can't fit on the text. Return undefined | ||
}; | ||
} | ||
|
||
// Search best fit offsets for each hunk based on the previous ones | ||
for (let i = 0; i < hunks.length; i++) { | ||
let hunk = hunks[i], | ||
maxLine = lines.length - hunk.oldLines, | ||
localOffset = 0, | ||
toPos = offset + hunk.oldStart - 1; | ||
|
||
let iterator = distanceIterator(toPos, minLine, maxLine); | ||
|
||
for (; localOffset !== undefined; localOffset = iterator()) { | ||
if (hunkFits(hunk, toPos + localOffset)) { | ||
hunk.offset = offset += localOffset; | ||
break; | ||
} | ||
} | ||
|
||
if (localOffset === undefined) { | ||
return false; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What behavior does this change that is necessary to do two iterations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is WiP to allow to calculate the location offset where the hunks best fit. This is to fix issue #84 |
||
} | ||
|
||
// Set lower text limit to end of the current hunk, so next ones don't try | ||
// to fit over already patched text | ||
minLine = hunk.offset + hunk.oldStart + hunk.oldLines; | ||
} | ||
|
||
// Apply patch hunks | ||
for (let i = 0; i < hunks.length; i++) { | ||
let hunk = hunks[i], | ||
toPos = hunk.offset + hunk.newStart - 1; | ||
|
||
for (let j = 0; j < hunk.lines.length; j++) { | ||
let line = hunk.lines[j], | ||
operation = line[0], | ||
content = line.substr(1); | ||
|
||
if (operation === ' ') { | ||
toPos++; | ||
|
@@ -84,7 +169,7 @@ export function applyPatches(uniDiff, options) { | |
function processIndex() { | ||
let index = uniDiff[currentIndex++]; | ||
if (!index) { | ||
options.complete(); | ||
return options.complete(); | ||
} | ||
|
||
options.loadFile(index, function(err, data) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As long as the tests pass I'm fine taking version changes as is, but generally it's better to keep the changes in a PR down to just those needed to implement your feature. Makes it easier to review and less likely to be rejected for reasons that are tangential to your feature change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. When I have it finished I can be able to split the commits in several feature branches so we can analize them separately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've cherry-picked these into master if you want to rebase to help simplify the PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I've rebased it.