Merge pull request #227 from github/line-numbers

Add line numbers to diffs

prototype/Doubt/JSONParser.swift

@@ -10,11 +10,11 @@ import Foundation
 import Madness
 import Prelude
 
-public typealias CofreeJSON = Cofree<JSONLeaf, Range<String.CharacterView.Index>>
+public typealias CofreeJSON = Cofree<JSONLeaf, (Range<Line>, Range<Column>, Range<String.Index>)>
 public typealias JSONParser = Parser<String.CharacterView, CofreeJSON>.Function
 
 // Inlined for performance reasons
-let whitespace = oneOf(" \n\r\t")*
+let whitespace = many(oneOf(" \n\r\t"))
 
 // TODO: Parse unicode escape sequence
 let escapeChar: StringParser = curry(+) <^> %"\\" <*> ({ String($0) } <^> oneOf("\\\"bfnrt"))
@@ -29,16 +29,16 @@ typealias MembersParser = Parser<String.CharacterView, [(String, CofreeJSON)]>.F
 
 // Parses an array of (String, CofreeJSON) object members
 func members(json: JSONParser) -> MembersParser {
-	let keyAndKeyTerm: Parser<String.CharacterView, (String, CofreeJSON)>.Function = quoted --> { (_, range, key) in
+	let keyAndKeyTerm: Parser<String.CharacterView, (String, CofreeJSON)>.Function = quoted --> { _, lines, columns, range, key in
-		(key, Cofree(range, .Leaf(.String(key))))
+		(key, Cofree((lines, columns, range), .Leaf(.String(key))))
 	}
 	let pairs: Parser<String.CharacterView, (String, CofreeJSON)>.Function = (curry(pair) <^>
 		keyAndKeyTerm
 		<* whitespace
 		<* %":"
 		<* whitespace
-		<*> json) --> { (_, range, values) in
+		<*> json) --> { _, lines, columns, range, values in
-			(values.0.0, Cofree(range, .Fixed([values.0.1, values.1])))
+			(values.0.0, Cofree((lines, columns, range), .Fixed([values.0.1, values.1])))
 		}
 
 	return sepBy(pairs, whitespace <* %"," <* whitespace)
@@ -46,7 +46,7 @@ func members(json: JSONParser) -> MembersParser {
 
 public let json: JSONParser = fix { json in
 	let string: JSONParser = quoted --> {
-		Cofree($1, .Leaf(.String($2)))
+		Cofree(($1, $2, $3), .Leaf(.String($4)))
 	} <?> "string"
 
 	let array: JSONParser =  %"["
@@ -55,7 +55,7 @@ public let json: JSONParser = fix { json in
 		<* whitespace
 		<* %"]"
 		--> {
-			Cofree($1, .Indexed($2))
+			Cofree(($1, $2, $3), .Indexed($4))
 		} <?> "array"
 
 	let object: JSONParser = %"{"
@@ -63,21 +63,21 @@ public let json: JSONParser = fix { json in
 		*> members(json)
 		<* whitespace
 		<* %"}"
-		--> { (_, range, values: [(String, CofreeJSON)]) in
+		--> { (_, lines, columns, range, values: [(String, CofreeJSON)]) in
-			Cofree(range, .Keyed(Dictionary(elements: values)))
+			Cofree((lines, columns, range), .Keyed(Dictionary(elements: values)))
 		} <?> "object"
 
-	let numberParser: JSONParser = (number --> { _, range, value in
+	let numberParser: JSONParser = (number --> { _, lines, columns, range, value in
-		Cofree(range, .Leaf(JSONLeaf.Number(value)))
+		Cofree((lines, columns, range), .Leaf(JSONLeaf.Number(value)))
 	}) <?> "number"
 	
-	let null: JSONParser = %"null" --> { (_, range, value) in
+	let null: JSONParser = %"null" --> { _, lines, columns, range, value in
-		return Cofree(range, .Leaf(.Null))
+		return Cofree((lines, columns, range), .Leaf(.Null))
 	} <?> "null"
 	
-	let boolean: JSONParser = %"false" <|> %"true" --> { (_, range, value) in
+	let boolean: JSONParser = %"false" <|> %"true" --> { _, lines, columns, range, value in
 		let boolean = value == "true"
-		return Cofree(range, .Leaf(.Boolean(boolean)))
+		return Cofree((lines, columns, range), .Leaf(.Boolean(boolean)))
 	} <?> "boolean"
 
 	// TODO: This should be JSON = dict <|> array and

prototype/DoubtTests/JSONParserTests.swift

@@ -16,10 +16,10 @@ final class JSONParserTests: XCTestCase {
 
 		let expected: Cofree<JSONLeaf, Range<Int>> = Cofree(0..<42, .Keyed(["hello": fixedPairs]))
 		let actual = Madness.parse(json, input: dictWithArray).right!
-		let firstIndex = actual.extract
+		let startRange = actual.extract.2
-		let new: Cofree<JSONLeaf, Range<Int>> = actual.map({ range in
+		let new: Cofree<JSONLeaf, Range<Int>> = actual.map({ tuple in
-			let startI: Int = firstIndex.startIndex.distanceTo(range.startIndex)
+			let startI: Int = startRange.startIndex.distanceTo(tuple.2.startIndex)
-			let endI: Int = firstIndex.startIndex.distanceTo(range.endIndex)
+			let endI: Int = startRange.startIndex.distanceTo(tuple.2.endIndex)
 			return Range(start: startI, end: endI)
 		})
 		

prototype/External/Madness

@@ -1 +1 @@
-Subproject commit 68fcabcdecd9219f3b3de0f423ff5e08aaaffae8
+Subproject commit 5f4f5f518f2ac881ee1ab4c188f111af3df92163

prototype/UI/diff.js

@@ -129,7 +129,7 @@ function termToDOM(source, syntax, extract, getRange, recur) {
 }
 
 /// Diff -> String -> DOM
-function diffToDOM(diff, sources) {
+function diffToDOM(diff, sources, lineNumbers) {
 
 	function getRange(diffOrTerm) {
 		if (diffOrTerm.pure != null) {
@@ -160,17 +160,17 @@ function diffToDOM(diff, sources) {
 	}
 
 	if (diff.pure != null) {
-		return pureToDOM(sources, diff.pure, getRange, function(diff) {
+		return pureToDOM(sources, diff.pure, lineNumbers, getRange, function(diff) {
-			return diffToDOM(diff, sources);
+			return diffToDOM(diff, sources, lineNumbers);
 		})
 	}
 
-	return rollToDOM(sources, diff.roll, getRange, function(diff) {
+	return rollToDOM(sources, diff.roll, lineNumbers, getRange, function(diff) {
-		return diffToDOM(diff, sources);
+		return diffToDOM(diff, sources, lineNumbers);
 	})
 }
 
-function pureToDOM(sources, patch, getRangeFun, diffToDOMFun) {
+function pureToDOM(sources, patch, lineNumbers, getRangeFun, diffToDOMFun) {
 	var elementA, elementB;
 	if (patch.before != null) {
 		elementA = termToDOM(sources.before, patch.before.unwrap, patch.before.extract, getRangeFun);
@@ -178,6 +178,8 @@ function pureToDOM(sources, patch, getRangeFun, diffToDOMFun) {
 		if (patch.after != null) {
 			elementA.classList.add("replace");
 		}
+
+		elementA.setAttribute("data-line-number", patch.before.extract.lines[0])
 	}
 
 	if (patch.after != null) {
@@ -186,22 +188,28 @@ function pureToDOM(sources, patch, getRangeFun, diffToDOMFun) {
 		if (patch.before != null) {
 			elementB.classList.add("replace");
 		}
+
+		elementB.setAttribute("data-line-number", patch.after.extract.lines[0])
 	}
 
 	if (elementA == null) {
-		elementA = elementB.cloneNode(true)
+		elementA = elementB.cloneNode(true);
-		elementA.classList.add("invisible")
+		elementA.classList.add("invisible");
+
+		elementA.setAttribute("data-line-number", '\u00A0')
 	}
 
 	if (elementB == null) {
-		elementB = elementA.cloneNode(true)
+		elementB = elementA.cloneNode(true);
-		elementB.classList.add("invisible")
+		elementB.classList.add("invisible");
+
+		elementB.setAttribute("data-line-number", '\u00A0')
 	}
 
 	return { "before": elementA || "", "after": elementB || "" };
 }
 
-function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
+function rollToDOM(sources, rollOrTerm, lineNumbers, getRangeFun, diffToDOMFun) {
 	var syntax = rollOrTerm.unwrap
 	var categories = {
 		before: rollOrTerm.extract.before.categories,
@@ -212,6 +220,11 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 		after: rollOrTerm.extract.after.range
 	}
 
+	var lines = {
+		before: rollOrTerm.extract.before.lines[0],
+		after: rollOrTerm.extract.after.lines[0]
+	}
+
 	var elementA;
 	var elementB;
 	if (syntax.leaf != null) {
@@ -219,12 +232,17 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 		elementA.textContent = sources.before.substr(range.before[0], range.before[1]);
 		elementB = document.createElement("span");
 		elementB.textContent = sources.after.substr(range.after[0], range.after[1]);
+
+		elementA.setAttribute("data-line-number", lines.before)
+		elementB.setAttribute("data-line-number", lines.after)
 	} else if (syntax.indexed != null || syntax.fixed != null) {
 		var values = syntax.indexed || syntax.fixed;
 		elementA = document.createElement("ul");
 		elementB = document.createElement("ul");
 		var previousBefore = range.before[0];
 		var previousAfter = range.after[0];
+
+		var lineNumbers = { "before": [], "after": [] };
 		for (i in values) {
 			var child = values[i];
 			if (child.pure == "") continue;
@@ -243,6 +261,11 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 					previousBefore = beforeRange[0] + beforeRange[1];
 				}
 				elementA.appendChild(li);
+
+				var lineNumber = beforeAfterChild.before.getAttribute("data-line-number")
+				if (lineNumber != null) {
+					lineNumbers.before.push(lineNumber)
+				}
 			}
 			if (childRange.after != null) {
 				var afterRange = childRange.after;
@@ -256,6 +279,11 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 					previousAfter = afterRange[0] + afterRange[1];
 				}
 				elementB.appendChild(li);
+
+				var lineNumber = beforeAfterChild.after.getAttribute("data-line-number");
+				if (lineNumber != null) {
+					lineNumbers.after.push(lineNumber)
+				}
 			}
 		}
 		var beforeText = sources.before.substr(previousBefore, range.before[0] + range.before[1] - previousBefore);
@@ -263,6 +291,9 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 
 		elementA.appendChild(document.createTextNode(beforeText));
 		elementB.appendChild(document.createTextNode(afterText));
+
+		elementA.setAttribute("data-line-number", lineNumbers.before)
+		elementB.setAttribute("data-line-number", lineNumbers.after)
 	} else if (syntax.keyed != null) {
 		elementA = document.createElement("dl");
 		elementB = document.createElement("dl");
@@ -330,6 +361,8 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 		var previousA = range.before[0];
 		var previousB = range.after[0];
 
+		var lineNumbers = { "before": [ lines.before ], "after": [ lines.after ] };
+
 		zip(befores, afters, function (a, b) {
 			var key = a.key
 			var childElA = a.child
@@ -388,6 +421,11 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 				ddA.classList.add("invisible");
 			}
 
+			var lineNumberA = childElA.getAttribute("data-line-number");
+			if (lineNumberA != null) {
+				lineNumbers.before.push(lineNumberA)
+			}
+
 			var dtB = wrap("dt", document.createTextNode(key));
 			elementB.appendChild(dtB);
 			var ddB = wrap("dd", childElB);
@@ -397,6 +435,10 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 				ddB.classList.add("invisible");
 			}
 
+			var lineNumberB = childElB.getAttribute("data-line-number");
+			if (lineNumberB != null) {
+				lineNumbers.after.push(lineNumberB)
+			}
 
 			if (isFirst || !childElA.classList.contains("invisible")) {
 				previousA = childRangeA[0] + childRangeA[1]
@@ -440,6 +482,12 @@ function rollToDOM(sources, rollOrTerm, getRangeFun, diffToDOMFun) {
 		elementA.appendChild(document.createTextNode(textA));
 		var textB = sources.after.substr(previousB, range.after[0] + range.after[1] - previousB);
 		elementB.appendChild(document.createTextNode(textB));
+
+
+		lineNumbers.before.push(rollOrTerm.extract.before.lines[1])
+		lineNumbers.after.push(rollOrTerm.extract.after.lines[1])
+		elementA.setAttribute("data-line-number", lineNumbers.before)
+		elementB.setAttribute("data-line-number", lineNumbers.after)
 	}
 
 	for (index in categories.before) {

prototype/UI/index.html

@@ -43,6 +43,13 @@
         color: initial;
     }
 
+		ul.line-numbers {
+			list-style: none;
+			margin: 0;
+			padding: 0;
+			float: left;
+		}
+
     .diff dt {
         display: none;
     }
@@ -110,16 +117,50 @@
     </script>
   </head>
   <body>
-	<div id="before" class="diff"></div>
+	<div id="before">
-	<div id="after" class="diff"></div>
+		<ul id="before-lines" class="line-numbers"></ul>
+		<div id="before-diff" class="diff"></div>
+	</div>
+	<div id="after">
+		<ul id="after-lines" class="line-numbers"></ul>
+		<div id="after-diff" class="diff"></div>
+	</div>
 
-    <script type="text/javascript">
+	<script type="text/javascript">
-    loadJSON((window.location.search || '?diff.json').substr(1), function (json) {
+	var unique = function(array) {
-        var diff = diffFromJSON(json.diff);
+		return array.reduce(function(accum, current) {
-		var dom = diffToDOM(diff, { "before": json["before"] , "after": json["after"] })
+			if (accum.indexOf(current) < 0) {
-		document.getElementById("before").appendChild(dom.before);
+				accum.push(current);
-		document.getElementById("after").appendChild(dom.after);
+			}
-    });
+			return accum;
-    </script>
+		}, []);
-  </body>
+	}
-</html>
+
+	loadJSON((window.location.search || '?diff.json').substr(1), function (json) {
+		var diff = diffFromJSON(json.diff);
+
+		var beforeLinesEl = document.getElementById("before-lines")
+		var afterLinesEl = document.getElementById("after-lines")
+
+		var dom = diffToDOM(diff,
+			{ "before": json["before"] , "after": json["after"] },
+			{ "before": beforeLinesEl, "after": afterLinesEl });
+
+			var beforeLines = dom.before.getAttribute("data-line-number").split(",")
+			unique(beforeLines).forEach(function(lineNumber) {
+				var node = wrap("li", document.createTextNode(lineNumber));
+				beforeLinesEl.appendChild(node);
+			});
+			var afterLines = dom.after.getAttribute("data-line-number").split(",")
+			unique(afterLines).forEach(function(lineNumber) {
+				var node = wrap("li", document.createTextNode(lineNumber));
+				afterLinesEl.appendChild(node);
+			});
+
+
+			document.getElementById("before-diff").appendChild(dom.before);
+			document.getElementById("after-diff").appendChild(dom.after);
+		});
+		</script>
+	</body>
+	</html>

prototype/doubt-difftool/Info.swift

@@ -1,17 +1,25 @@
 struct Info: Categorizable, CustomJSONConvertible, Equatable {
-	init(range: Range<Int>, categories: Set<String>) {
+	init(range: Range<Int>, lines: Range<Line>, columns: Range<Column>, categories: Set<String>) {
 		self.range = range
+		self.lines = lines
+		self.columns = columns
 		self.categories = categories
 	}
 
-	init(range: Range<String.CharacterView.Index>, categories: Set<String>) {
+	init(range: Range<String.CharacterView.Index>, lines: Range<Line>, columns: Range<Column>, categories: Set<String>) {
 		// FIXME: this is terrible. see also https://github.com/github/semantic-diff/issues/136
 		self.range = Int(String(range.startIndex))!..<Int(String(range.endIndex))!
+		self.lines = lines
+		self.columns = columns
 		self.categories = categories
 	}
 
 	let range: Range<Int>
 
+	let lines: Range<Line>
+	
+	let columns: Range<Column>
+
 
 	// MARK: Categorizable
 
@@ -23,14 +31,16 @@ struct Info: Categorizable, CustomJSONConvertible, Equatable {
 	var JSON: Doubt.JSON {
 		return [
 			"range": range.JSON,
+			"lines": lines.JSON,
+			"columns": columns.JSON,
 			"categories": Array(categories).JSON
 		]
 	}
 }
 
 func == (left: Info, right: Info) -> Bool {
-	return left.range == right.range && left.categories == right.categories
+	return left.range == right.range && left.categories == right.categories && left.lines == left.lines && left.columns == right.columns
 }
 
-
+import Madness
 import Doubt

prototype/doubt-difftool/main.swift

@@ -93,16 +93,20 @@ func termWithInput(language: TSLanguage)(_ string: String) throws -> Term {
 				}
 			} (root, "program")
 			.map { node, category in
-				Info(range: node.range, categories: [ category ])
+				// TODO: Calculate line and column from TSNodes
+				Info(range: node.range, lines: 0..<1, columns: 0..<1, categories: [ category ])
 			}
 	}
 }
 
 func toTerm(term: CofreeJSON) -> Term {
-	let annotation = Info(range: term.extract, categories: [])
+	let lines = term.extract.0
+	let columns = term.extract.1
+	let range = term.extract.2
+	let annotation = Info(range: range, lines: lines, columns: columns, categories: [])
 	switch term.unwrap {
 	case let .Leaf(a):
-		return Term(Info(range: term.extract, categories: a.categories), Syntax<Term, String>.Leaf(String(a)))
+		return Term(Info(range: range, lines: lines, columns: columns, categories: a.categories), Syntax<Term, String>.Leaf(String(a)))
 	case let .Indexed(i):
 		return Term(annotation, .Indexed(i.map(toTerm)))
 	case let .Fixed(f):
@@ -115,14 +119,16 @@ func toTerm(term: CofreeJSON) -> Term {
 func lines(input: String) -> Term {
 	var lines: [Term] = []
 	var previous = 0
+	var lineNumber = 0
 	input.enumerateSubstringsInRange(input.characters.indices, options: .ByLines) { (line, _, enclosingRange, _) in
 		let range: Range<Int> = previous..<(previous + enclosingRange.count)
 		previous = range.endIndex
 		if let line = line {
-			lines.append(Term(Info(range: range, categories: []), Syntax.Leaf(line)))
+			lineNumber += 1
+			lines.append(Term(Info(range: range, lines: 0..<lineNumber, columns: 0..<1, categories: []), Syntax.Leaf(line)))
 		}
 	}
-	return Term(Info(range: 0..<input.utf16.count, categories: []), .Indexed(lines))
+	return Term(Info(range: 0..<input.utf16.count, lines: 0..<lineNumber, columns: 0..<1, categories: []), .Indexed(lines))
 }
 
 func parserForType(type: String) -> String throws -> Term {
@@ -154,8 +160,8 @@ extension ForwardIndexType {
 func refineLeafReplacement(aString: String, _ bString: String)(_ patch: Patch<Term>) -> Diff {
 	switch patch {
 	case let .Replace(.Unroll(aExtract, .Leaf), .Unroll(bExtract, .Leaf)):
-		let a = aString.utf16[aExtract.range].enumerate().map { Term(Info(range: (aExtract.range.startIndex + $0).range, categories: aExtract.categories), .Leaf(String($1))) }
+		let a = aString.utf16[aExtract.range].enumerate().map { Term(Info(range: (aExtract.range.startIndex + $0).range, lines: aExtract.lines, columns: aExtract.columns, categories: aExtract.categories), .Leaf(String($1))) }
-		let b = bString.utf16[bExtract.range].enumerate().map { Term(Info(range: (bExtract.range.startIndex + $0).range, categories: bExtract.categories), .Leaf(String($1))) }
+		let b = bString.utf16[bExtract.range].enumerate().map { Term(Info(range: (bExtract.range.startIndex + $0).range, lines: bExtract.lines, columns: bExtract.columns, categories: bExtract.categories), .Leaf(String($1))) }
 		return .Roll((aExtract, bExtract), .Indexed(SES(a, b, cost: Diff.sum(Patch.sum), recur: { Term.equals(annotation: const(true), leaf: ==)($0, $1) ? Term.zip($0, $1).map(Diff.init) : Diff.Replace($0, $1) })))
 	default:
 		return .Pure(patch)