Allow patchs in diff format #83
There are no files selected for viewing
| 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; | ||
|
|
||
| /** | ||
| * 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. 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()) { | ||
|
There was a problem hiding this comment. You need There was a problem hiding this comment.
[piranna@Mabuk:~/Proyectos/NodeOS/node_modules]
(vagga) > node
> 0 != undefined
true
> I think it's not needed... :-) But I could agree on the
A test case for what, exactly? There was a problem hiding this comment. Perhaps it's been too long that I've been using Little bit of a nit picky change but would be nice to have so it doesn't cause confusion down the road. Test case: To make sure that it can patch at an offset of zero. |
||
| if (hunkFits(hunk, toPos + localOffset)) { | ||
| hunk.offset = offset += localOffset; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (localOffset == undefined) { | ||
| return false; | ||
|
There was a problem hiding this comment. What behavior does this change that is necessary to do two iterations? There was a problem hiding this comment. 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) { | ||
| 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.