Skip to content

Commit 232dbb1

Browse files
johnhaley81johnhaley81
committed
Merge pull request #844 from nodegit/fix-line-length
Modify `ConvenientLine` to work with unicode
2 parents 76a08dc + cff2a0a commit 232dbb1

6 files changed

Lines changed: 113 additions & 45 deletions

File tree

‎lib/convenient_line.js‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3,59 +3,59 @@ var NodeGit = require("../");
33
function ConvenientLine(raw, i) {
44
this.raw = raw;
55
this.i = i;
6+
this._cache = {};
67
}
78

89
/**
9-
* The content of the line
10-
* @return {String}
10+
* The type of the line (context, addition, deletion, etc...)
11+
* @return {Diff.LINE}
1112
*/
1213
ConvenientLine.prototype.origin = function() {
1314
return this.raw.origin();
1415
};
1516

1617
/**
17-
* The content of the line
18-
* @return {String}
18+
* Line number in old file or -1 for added line
19+
* @return {Number}
1920
*/
2021
ConvenientLine.prototype.oldLineno = function() {
2122
return this.raw.oldLineno();
2223
};
2324

2425
/**
25-
* The content of the line
26-
* @return {String}
26+
* Line number in new file or -1 for deleted line
27+
* @return {Number}
2728
*/
2829
ConvenientLine.prototype.newLineno = function() {
2930
return this.raw.newLineno();
3031
};
3132

3233
/**
33-
* The content of the line
34-
* @return {String}
34+
* Number of newline characters in content
35+
* @return {Number}
3536
*/
3637
ConvenientLine.prototype.numLines = function() {
3738
return this.raw.numLines();
3839
};
3940

4041
/**
41-
* The content of the line
42-
* @return {String}
42+
* Number of bytes in the string
43+
* @return {Number}
4344
*/
4445
ConvenientLine.prototype.contentLen = function() {
45-
return this.raw.contentLen();
46+
return this.raw.contentLen();
4647
};
4748

48-
4949
/**
50-
* The content of the line
51-
* @return {String}
50+
* Offset in the original file to the content
51+
* @return {Number}
5252
*/
5353
ConvenientLine.prototype.contentOffset = function() {
5454
return this.raw.contentOffset();
5555
};
5656

5757
/**
58-
* The content of the line
58+
* Pointer to diff text, not NUL-byte terminated
5959
* @return {String}
6060
*/
6161
ConvenientLine.prototype.rawContent = function() {
@@ -67,8 +67,13 @@ ConvenientLine.prototype.rawContent = function() {
6767
* @return {String}
6868
*/
6969
ConvenientLine.prototype.content = function() {
70-
return this.raw.content()
71-
.substring(0, this.raw.contentLen()).replace("\n", "");
70+
if (!this._cache.content) {
71+
this._cache.content = new Buffer(this.raw.content())
72+
.slice(0, this.raw.contentLen())
73+
.toString("utf8");
74+
}
75+
76+
return this._cache.content;
7277
};
7378

7479
NodeGit.ConvenientLine = ConvenientLine;

‎lib/repository.js‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1286,9 +1286,6 @@ Repository.prototype.stageLines =
12861286
(!isStaged && (!newLine || newLine.length === 0))) {
12871287
if (hunkLine.origin() !== lineTypes.ADDED) {
12881288
newContent += hunkLine.content();
1289-
if (hunkLine.raw.numLines() !== 0) {
1290-
newContent += "\n";
1291-
}
12921289
}
12931290
if ((isStaged && hunkLine.origin() !== lineTypes.DELETED) ||
12941291
(!isStaged && hunkLine.origin() !== lineTypes.ADDED)) {
@@ -1299,9 +1296,6 @@ Repository.prototype.stageLines =
12991296
switch (hunkLine.origin()) {
13001297
case lineTypes.ADDED:
13011298
newContent += hunkLine.content();
1302-
if (hunkLine.raw.numLines() !== 0) {
1303-
newContent += "\n";
1304-
}
13051299
if (isStaged) {
13061300
oldIndex++;
13071301
}

‎test/tests/commit.js‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -520,10 +520,6 @@ describe("Commit", function() {
520520
" line q\nline r\nline s\nline t\n\nline u\n" +
521521
"line v1\nline w\nline x\n \nline y\nline z\n";
522522

523-
function getLineText(line) {
524-
return line.rawContent().substring(0, line.contentLen());
525-
}
526-
527523
return NodeGit.Repository.open(reposPath)
528524
.then(function(repoResult) {
529525
repo = repoResult;
@@ -559,33 +555,37 @@ describe("Commit", function() {
559555
//check all hunk lines
560556
assert.equal(lines.length, 12);
561557
assert.equal(lines[0].origin(), Diff.LINE.CONTEXT);
562-
assert.equal(lines[1].contentLen(), 9);
563558

564-
assert.equal(getLineText(lines[1]), "line s\n");
559+
assert.equal(lines[1].content().length, 9);
560+
assert.equal(lines[1].content(), "line s\n");
565561
assert.equal(lines[1].origin(), Diff.LINE.CONTEXT);
562+
566563
assert.equal(lines[2].origin(), Diff.LINE.CONTEXT);
567564

568-
assert.equal(lines[3].contentLen(), 1);
569-
assert.equal(getLineText(lines[3]), "\n");
565+
assert.equal(lines[3].content().length, 1);
566+
assert.equal(lines[3].content(), "\n");
570567
assert.equal(lines[3].origin(), Diff.LINE.ADDITION);
571568

572569
assert.equal(lines[4].origin(), Diff.LINE.CONTEXT);
573570

574-
assert.equal(lines[5].contentLen(), 7);
575-
assert.equal(getLineText(lines[5]), "line v\n");
571+
assert.equal(lines[5].content().length, 7);
572+
assert.equal(lines[5].content(), "line v\n");
576573
assert.equal(lines[5].origin(), Diff.LINE.DELETION);
577-
assert.equal(lines[6].contentLen(), 8);
578-
assert.equal(getLineText(lines[6]), "line v1\n");
574+
575+
assert.equal(lines[6].content().length, 8);
576+
assert.equal(lines[6].content(), "line v1\n");
579577
assert.equal(lines[6].origin(), Diff.LINE.ADDITION);
580578

581579
assert.equal(lines[7].origin(), Diff.LINE.CONTEXT);
580+
582581
assert.equal(lines[8].origin(), Diff.LINE.CONTEXT);
583582

584-
assert.equal(lines[9].contentLen(), 4);
585-
assert.equal(getLineText(lines[9]), "\t\t\t\n");
583+
assert.equal(lines[9].content().length, 4);
584+
assert.equal(lines[9].content(), "\t\t\t\n");
586585
assert.equal(lines[9].origin(), Diff.LINE.ADDITION);
587586

588587
assert.equal(lines[10].origin(), Diff.LINE.CONTEXT);
588+
589589
assert.equal(lines[11].origin(), Diff.LINE.CONTEXT);
590590
});
591591
});

‎test/tests/convenient_line.js‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
var assert = require("assert");
2+
var repoSetup = require("../utils/repository_setup");
3+
var promisify = require("promisify-node");
4+
var fse = promisify(require("fs-extra"));
5+
var path = require("path");
6+
var local = path.join.bind(path, __dirname);
7+
8+
describe("ConvenientLine", function() {
9+
var repoPath = local("../repos/convenientLineTest");
10+
var unicodeLine = "Ťḥ𝖎ṧ ℓỈ𝓃ệ çǒ𝚗ẗảḭṋṦ Û𝐧ǐ𝗰ṓḍ𝔢\n";
11+
var asciiLine = "but this line doesn't\n";
12+
13+
beforeEach(function() {
14+
var test = this;
15+
16+
return repoSetup.createRepository(repoPath)
17+
.then(function(repo) {
18+
return repoSetup.commitFileToRepo(
19+
repo,
20+
"fileWithUnicodeChars",
21+
unicodeLine + asciiLine
22+
);
23+
})
24+
.then(function(commit) {
25+
return commit.getDiff();
26+
})
27+
.then(function(diff) {
28+
return diff[0].patches();
29+
})
30+
.then(function(patches) {
31+
return patches[0].hunks();
32+
})
33+
.then(function(hunks) {
34+
return hunks[0].lines();
35+
})
36+
.then(function(lines) {
37+
test.unicodeLine = lines[0];
38+
test.asciiLine = lines[1];
39+
});
40+
});
41+
42+
after(function() {
43+
return fse.remove(repoPath);
44+
});
45+
46+
it("can parse the byte length of a unicode string", function() {
47+
var line = this.unicodeLine;
48+
49+
assert.equal(line.contentLen(), Buffer.byteLength(unicodeLine, "utf8"));
50+
});
51+
52+
it("can get a line that contains unicode", function() {
53+
var line = this.unicodeLine;
54+
55+
assert.equal(line.content(), unicodeLine);
56+
});
57+
58+
it("can parse the byte length of a ascii string", function() {
59+
var line = this.asciiLine;
60+
61+
assert.equal(line.contentLen(), Buffer.byteLength(asciiLine, "utf8"));
62+
});
63+
64+
it("can get a line that contains ascii", function() {
65+
var line = this.asciiLine;
66+
67+
assert.equal(line.content(), asciiLine);
68+
});
69+
});

‎test/tests/diff.js‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,16 +115,16 @@ describe("Diff", function() {
115115

116116
var oldContent = "__Before submitting a pull request, please ensure " +
117117
"both unit tests and lint checks pass.__\n";
118-
assert.equal(lines[3].rawContent(), oldContent);
118+
assert.equal(lines[3].content(), oldContent);
119119
assert.equal(lines[3].origin(), Diff.LINE.DELETION);
120-
assert.equal(lines[3].contentLen(), 90);
120+
assert.equal(lines[3].content().length, oldContent.length);
121121

122122
var newContent = "__Before submitting a pull request, please ensure " +
123123
"both that you've added unit tests to cover your shiny new code, " +
124124
"and that all unit tests and lint checks pass.__\n";
125-
assert.equal(lines[4].rawContent(), newContent);
125+
assert.equal(lines[4].content(), newContent);
126126
assert.equal(lines[4].origin(), Diff.LINE.ADDITION);
127-
assert.equal(lines[4].contentLen(), 162);
127+
assert.equal(lines[4].content().length, newContent.length);
128128
});
129129
});
130130

@@ -182,7 +182,7 @@ describe("Diff", function() {
182182
})
183183
.then(function(lines) {
184184
lines.forEach(function(line) {
185-
assert(!/\n/.exec(line.content()));
185+
assert(/\n/.exec(line.content()));
186186
assert(/\n/.exec(line.rawContent()));
187187
});
188188
});

‎test/tests/stage.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,7 @@ function stagingTest(staging, newFileContent) {
245245
throw ("File change when no file change expected.");
246246
}
247247
} else {
248-
assert(delta.newFile().mode() - delta.oldFile().mode() ===
248+
assert(delta.newFile().mode() - delta.oldFile().mode() ===
249249
fileModeDifference);
250250
}
251251
return true;
@@ -265,7 +265,7 @@ function stagingTest(staging, newFileContent) {
265265
fileContent)
266266
.then(function() {
267267
//Then, change the permission locally.
268-
return fse.chmod(path.join(test.repository.workdir(), fileName),
268+
return fse.chmod(path.join(test.repository.workdir(), fileName),
269269
0755 /* new filemode */);
270270
});
271271
})
@@ -310,7 +310,7 @@ function stagingTest(staging, newFileContent) {
310310
fileContent)
311311
.then(function() {
312312
//Then, change the permission locally.
313-
return fse.chmod(path.join(test.repository.workdir(), fileName),
313+
return fse.chmod(path.join(test.repository.workdir(), fileName),
314314
0755 /* new filemode */);
315315
});
316316
})

0 commit comments

Comments
 (0)