diff --git a/CHANGELOG.md b/CHANGELOG.md index 088a96697..a4ffeacd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ * Added popout diff visualizer for table properties like Attributes and Tags ([#834]) * Updated Theme to use Studio colors ([#838]) * Improved patch visualizer UX ([#883]) +* Improved script source diff visualizer UX and performance ([#994]) * Added update notifications for newer compatible versions in the Studio plugin. ([#832]) * Added experimental setting for Auto Connect in playtests ([#840]) * Improved settings UI ([#886]) @@ -91,6 +92,7 @@ [#974]: https://github.com/rojo-rbx/rojo/pull/974 [#987]: https://github.com/rojo-rbx/rojo/pull/987 [#988]: https://github.com/rojo-rbx/rojo/pull/988 +[#994]: https://github.com/rojo-rbx/rojo/pull/994 ## [7.4.3] - August 6th, 2024 * Fixed issue with building binary files introduced in 7.4.2 diff --git a/assets/images/diagonal-lines.png b/assets/images/diagonal-lines.png new file mode 100644 index 000000000..5e12b36de Binary files /dev/null and b/assets/images/diagonal-lines.png differ diff --git a/plugin/Packages/Highlighter b/plugin/Packages/Highlighter index e0d061449..d7473a878 160000 --- a/plugin/Packages/Highlighter +++ b/plugin/Packages/Highlighter @@ -1 +1 @@ -Subproject commit e0d061449ea5c4452ef77008b5197ae4d3d77621 +Subproject commit d7473a87807f30cdb5ee7268a560b5c33f22da67 diff --git a/plugin/src/App/Components/CodeLabel.lua b/plugin/src/App/Components/CodeLabel.lua deleted file mode 100644 index 1951106b9..000000000 --- a/plugin/src/App/Components/CodeLabel.lua +++ /dev/null @@ -1,66 +0,0 @@ -local Rojo = script:FindFirstAncestor("Rojo") -local Plugin = Rojo.Plugin -local Packages = Rojo.Packages - -local Roact = require(Packages.Roact) -local Highlighter = require(Packages.Highlighter) -Highlighter.matchStudioSettings() - -local e = Roact.createElement - -local Theme = require(Plugin.App.Theme) - -local CodeLabel = Roact.PureComponent:extend("CodeLabel") - -function CodeLabel:init() - self.labelRef = Roact.createRef() - self.highlightsRef = Roact.createRef() -end - -function CodeLabel:didMount() - Highlighter.highlight({ - textObject = self.labelRef:getValue(), - }) - self:updateHighlights() -end - -function CodeLabel:didUpdate() - self:updateHighlights() -end - -function CodeLabel:updateHighlights() - local highlights = self.highlightsRef:getValue() - if not highlights then - return - end - - for _, lineLabel in highlights:GetChildren() do - local lineNum = tonumber(string.match(lineLabel.Name, "%d+") or "0") - lineLabel.BackgroundColor3 = self.props.lineBackground - lineLabel.BorderSizePixel = 0 - lineLabel.BackgroundTransparency = if self.props.markedLines[lineNum] then 0.25 else 1 - end -end - -function CodeLabel:render() - return Theme.with(function(theme) - return e("TextLabel", { - Size = self.props.size, - Position = self.props.position, - Text = self.props.text, - BackgroundTransparency = 1, - FontFace = theme.Font.Code, - TextSize = theme.TextSize.Code, - TextXAlignment = Enum.TextXAlignment.Left, - TextYAlignment = Enum.TextYAlignment.Top, - TextColor3 = Color3.fromRGB(255, 255, 255), - [Roact.Ref] = self.labelRef, - }, { - SyntaxHighlights = e("Folder", { - [Roact.Ref] = self.highlightsRef, - }), - }) - end) -end - -return CodeLabel diff --git a/plugin/src/App/Components/StringDiffVisualizer/StringDiff.lua b/plugin/src/App/Components/StringDiffVisualizer/StringDiff.lua index 6632c006a..f8986895d 100644 --- a/plugin/src/App/Components/StringDiffVisualizer/StringDiff.lua +++ b/plugin/src/App/Components/StringDiffVisualizer/StringDiff.lua @@ -1,3 +1,4 @@ +--!strict --[[ Based on DiffMatchPatch by Neil Fraser. https://github.com/google/diff-match-patch @@ -67,11 +68,180 @@ function StringDiff.findDiffs(text1: string, text2: string): Diffs end -- Cleanup the diff + diffs = StringDiff._cleanupSemantic(diffs) diffs = StringDiff._reorderAndMerge(diffs) return diffs end +function StringDiff._computeDiff(text1: string, text2: string): Diffs + -- Assumes that the prefix and suffix have already been trimmed off + -- and shortcut returns have been made so these texts must be different + + local text1Length, text2Length = #text1, #text2 + + if text1Length == 0 then + -- It's simply inserting all of text2 into text1 + return { { actionType = StringDiff.ActionTypes.Insert, value = text2 } } + end + + if text2Length == 0 then + -- It's simply deleting all of text1 + return { { actionType = StringDiff.ActionTypes.Delete, value = text1 } } + end + + local longText = if text1Length > text2Length then text1 else text2 + local shortText = if text1Length > text2Length then text2 else text1 + local shortTextLength = #shortText + + -- Shortcut if the shorter string exists entirely inside the longer one + local indexOf = if shortTextLength == 0 then nil else string.find(longText, shortText, 1, true) + if indexOf ~= nil then + local diffs = { + { actionType = StringDiff.ActionTypes.Insert, value = string.sub(longText, 1, indexOf - 1) }, + { actionType = StringDiff.ActionTypes.Equal, value = shortText }, + { actionType = StringDiff.ActionTypes.Insert, value = string.sub(longText, indexOf + shortTextLength) }, + } + -- Swap insertions for deletions if diff is reversed + if text1Length > text2Length then + diffs[1].actionType, diffs[3].actionType = StringDiff.ActionTypes.Delete, StringDiff.ActionTypes.Delete + end + return diffs + end + + if shortTextLength == 1 then + -- Single character string + -- After the previous shortcut, the character can't be an equality + return { + { actionType = StringDiff.ActionTypes.Delete, value = text1 }, + { actionType = StringDiff.ActionTypes.Insert, value = text2 }, + } + end + + return StringDiff._bisect(text1, text2) +end + +function StringDiff._cleanupSemantic(diffs: Diffs): Diffs + -- Reduce the number of edits by eliminating semantically trivial equalities. + local changes = false + local equalities = {} -- Stack of indices where equalities are found. + local equalitiesLength = 0 -- Keeping our own length var is faster. + local lastEquality: string? = nil + -- Always equal to diffs[equalities[equalitiesLength]].value + local pointer = 1 -- Index of current position. + -- Number of characters that changed prior to the equality. + local length_insertions1 = 0 + local length_deletions1 = 0 + -- Number of characters that changed after the equality. + local length_insertions2 = 0 + local length_deletions2 = 0 + + while diffs[pointer] do + if diffs[pointer].actionType == StringDiff.ActionTypes.Equal then -- Equality found. + equalitiesLength = equalitiesLength + 1 + equalities[equalitiesLength] = pointer + length_insertions1 = length_insertions2 + length_deletions1 = length_deletions2 + length_insertions2 = 0 + length_deletions2 = 0 + lastEquality = diffs[pointer].value + else -- An insertion or deletion. + if diffs[pointer].actionType == StringDiff.ActionTypes.Insert then + length_insertions2 = length_insertions2 + #diffs[pointer].value + else + length_deletions2 = length_deletions2 + #diffs[pointer].value + end + -- Eliminate an equality that is smaller or equal to the edits on both + -- sides of it. + if + lastEquality + and (#lastEquality <= math.max(length_insertions1, length_deletions1)) + and (#lastEquality <= math.max(length_insertions2, length_deletions2)) + then + -- Duplicate record. + table.insert( + diffs, + equalities[equalitiesLength], + { actionType = StringDiff.ActionTypes.Delete, value = lastEquality } + ) + -- Change second copy to insert. + diffs[equalities[equalitiesLength] + 1].actionType = StringDiff.ActionTypes.Insert + -- Throw away the equality we just deleted. + equalitiesLength = equalitiesLength - 1 + -- Throw away the previous equality (it needs to be reevaluated). + equalitiesLength = equalitiesLength - 1 + pointer = (equalitiesLength > 0) and equalities[equalitiesLength] or 0 + length_insertions1, length_deletions1 = 0, 0 -- Reset the counters. + length_insertions2, length_deletions2 = 0, 0 + lastEquality = nil + changes = true + end + end + pointer = pointer + 1 + end + + -- Normalize the diff. + if changes then + StringDiff._reorderAndMerge(diffs) + end + StringDiff._cleanupSemanticLossless(diffs) + + -- Find any overlaps between deletions and insertions. + -- e.g: abcxxxxxxdef + -- -> abcxxxdef + -- e.g: xxxabcdefxxx + -- -> defxxxabc + -- Only extract an overlap if it is as big as the edit ahead or behind it. + pointer = 2 + while diffs[pointer] do + if + diffs[pointer - 1].actionType == StringDiff.ActionTypes.Delete + and diffs[pointer].actionType == StringDiff.ActionTypes.Insert + then + local deletion = diffs[pointer - 1].value + local insertion = diffs[pointer].value + local overlap_length1 = StringDiff._commonOverlap(deletion, insertion) + local overlap_length2 = StringDiff._commonOverlap(insertion, deletion) + if overlap_length1 >= overlap_length2 then + if overlap_length1 >= #deletion / 2 or overlap_length1 >= #insertion / 2 then + -- Overlap found. Insert an equality and trim the surrounding edits. + table.insert( + diffs, + pointer, + { actionType = StringDiff.ActionTypes.Equal, value = string.sub(insertion, 1, overlap_length1) } + ) + diffs[pointer - 1].value = string.sub(deletion, 1, #deletion - overlap_length1) + diffs[pointer + 1].value = string.sub(insertion, overlap_length1 + 1) + pointer = pointer + 1 + end + else + if overlap_length2 >= #deletion / 2 or overlap_length2 >= #insertion / 2 then + -- Reverse overlap found. + -- Insert an equality and swap and trim the surrounding edits. + table.insert( + diffs, + pointer, + { actionType = StringDiff.ActionTypes.Equal, value = string.sub(deletion, 1, overlap_length2) } + ) + diffs[pointer - 1] = { + actionType = StringDiff.ActionTypes.Insert, + value = string.sub(insertion, 1, #insertion - overlap_length2), + } + diffs[pointer + 1] = { + actionType = StringDiff.ActionTypes.Delete, + value = string.sub(deletion, overlap_length2 + 1), + } + pointer = pointer + 1 + end + end + pointer = pointer + 1 + end + pointer = pointer + 1 + end + + return diffs +end + function StringDiff._sharedPrefix(text1: string, text2: string): number -- Uses a binary search to find the largest common prefix between the two strings -- Performance analysis: http://neil.fraser.name/news/2007/10/09/ @@ -124,51 +294,162 @@ function StringDiff._sharedSuffix(text1: string, text2: string): number return pointerMid end -function StringDiff._computeDiff(text1: string, text2: string): Diffs - -- Assumes that the prefix and suffix have already been trimmed off - -- and shortcut returns have been made so these texts must be different - - local text1Length, text2Length = #text1, #text2 +function StringDiff._commonOverlap(text1: string, text2: string): number + -- Determine if the suffix of one string is the prefix of another. - if text1Length == 0 then - -- It's simply inserting all of text2 into text1 - return { { actionType = StringDiff.ActionTypes.Insert, value = text2 } } + -- Cache the text lengths to prevent multiple calls. + local text1_length = #text1 + local text2_length = #text2 + -- Eliminate the null case. + if text1_length == 0 or text2_length == 0 then + return 0 + end + -- Truncate the longer string. + if text1_length > text2_length then + text1 = string.sub(text1, text1_length - text2_length + 1) + elseif text1_length < text2_length then + text2 = string.sub(text2, 1, text1_length) + end + local text_length = math.min(text1_length, text2_length) + -- Quick check for the worst case. + if text1 == text2 then + return text_length end - if text2Length == 0 then - -- It's simply deleting all of text1 - return { { actionType = StringDiff.ActionTypes.Delete, value = text1 } } + -- Start by looking for a single character match + -- and increase length until no match is found. + -- Performance analysis: https://neil.fraser.name/news/2010/11/04/ + local best = 0 + local length = 1 + while true do + local pattern = string.sub(text1, text_length - length + 1) + local found = string.find(text2, pattern, 1, true) + if found == nil then + return best + end + length = length + found - 1 + if found == 1 or string.sub(text1, text_length - length + 1) == string.sub(text2, 1, length) then + best = length + length = length + 1 + end end +end - local longText = if text1Length > text2Length then text1 else text2 - local shortText = if text1Length > text2Length then text2 else text1 - local shortTextLength = #shortText +function StringDiff._cleanupSemanticScore(one: string, two: string): number + -- Given two strings, compute a score representing whether the internal + -- boundary falls on logical boundaries. + -- Scores range from 6 (best) to 0 (worst). - -- Shortcut if the shorter string exists entirely inside the longer one - local indexOf = if shortTextLength == 0 then nil else string.find(longText, shortText, 1, true) - if indexOf ~= nil then - local diffs = { - { actionType = StringDiff.ActionTypes.Insert, value = string.sub(longText, 1, indexOf - 1) }, - { actionType = StringDiff.ActionTypes.Equal, value = shortText }, - { actionType = StringDiff.ActionTypes.Insert, value = string.sub(longText, indexOf + shortTextLength) }, - } - -- Swap insertions for deletions if diff is reversed - if text1Length > text2Length then - diffs[1].actionType, diffs[3].actionType = StringDiff.ActionTypes.Delete, StringDiff.ActionTypes.Delete - end - return diffs + if (#one == 0) or (#two == 0) then + -- Edges are the best. + return 6 end - if shortTextLength == 1 then - -- Single character string - -- After the previous shortcut, the character can't be an equality - return { - { actionType = StringDiff.ActionTypes.Delete, value = text1 }, - { actionType = StringDiff.ActionTypes.Insert, value = text2 }, - } + -- Each port of this function behaves slightly differently due to + -- subtle differences in each language's definition of things like + -- 'whitespace'. Since this function's purpose is largely cosmetic, + -- the choice has been made to use each language's native features + -- rather than force total conformity. + local char1 = string.sub(one, -1) + local char2 = string.sub(two, 1, 1) + local nonAlphaNumeric1 = string.match(char1, "%W") + local nonAlphaNumeric2 = string.match(char2, "%W") + local whitespace1 = nonAlphaNumeric1 and string.match(char1, "%s") + local whitespace2 = nonAlphaNumeric2 and string.match(char2, "%s") + local lineBreak1 = whitespace1 and string.match(char1, "%c") + local lineBreak2 = whitespace2 and string.match(char2, "%c") + local blankLine1 = lineBreak1 and string.match(one, "\n\r?\n$") + local blankLine2 = lineBreak2 and string.match(two, "^\r?\n\r?\n") + + if blankLine1 or blankLine2 then + -- Five points for blank lines. + return 5 + elseif lineBreak1 or lineBreak2 then + -- Four points for line breaks + -- DEVIATION: Prefer to start on a line break instead of end on it + return if lineBreak1 then 4 else 4.5 + elseif nonAlphaNumeric1 and not whitespace1 and whitespace2 then + -- Three points for end of sentences. + return 3 + elseif whitespace1 or whitespace2 then + -- Two points for whitespace. + return 2 + elseif nonAlphaNumeric1 or nonAlphaNumeric2 then + -- One point for non-alphanumeric. + return 1 end + return 0 +end - return StringDiff._bisect(text1, text2) +function StringDiff._cleanupSemanticLossless(diffs: Diffs) + -- Look for single edits surrounded on both sides by equalities + -- which can be shifted sideways to align the edit to a word boundary. + -- e.g: The cat came. -> The cat came. + + local pointer = 2 + -- Intentionally ignore the first and last element (don't need checking). + while diffs[pointer + 1] do + local prevDiff, nextDiff = diffs[pointer - 1], diffs[pointer + 1] + if + (prevDiff.actionType == StringDiff.ActionTypes.Equal) + and (nextDiff.actionType == StringDiff.ActionTypes.Equal) + then + -- This is a single edit surrounded by equalities. + local diff = diffs[pointer] + + local equality1 = prevDiff.value + local edit = diff.value + local equality2 = nextDiff.value + + -- First, shift the edit as far left as possible. + local commonOffset = StringDiff._sharedSuffix(equality1, edit) + if commonOffset > 0 then + local commonString = string.sub(edit, -commonOffset) + equality1 = string.sub(equality1, 1, -commonOffset - 1) + edit = commonString .. string.sub(edit, 1, -commonOffset - 1) + equality2 = commonString .. equality2 + end + + -- Second, step character by character right, looking for the best fit. + local bestEquality1 = equality1 + local bestEdit = edit + local bestEquality2 = equality2 + local bestScore = StringDiff._cleanupSemanticScore(equality1, edit) + + StringDiff._cleanupSemanticScore(edit, equality2) + + while string.byte(edit, 1) == string.byte(equality2, 1) do + equality1 = equality1 .. string.sub(edit, 1, 1) + edit = string.sub(edit, 2) .. string.sub(equality2, 1, 1) + equality2 = string.sub(equality2, 2) + local score = StringDiff._cleanupSemanticScore(equality1, edit) + + StringDiff._cleanupSemanticScore(edit, equality2) + -- The >= encourages trailing rather than leading whitespace on edits. + if score >= bestScore then + bestScore = score + bestEquality1 = equality1 + bestEdit = edit + bestEquality2 = equality2 + end + end + if prevDiff.value ~= bestEquality1 then + -- We have an improvement, save it back to the diff. + if #bestEquality1 > 0 then + diffs[pointer - 1].value = bestEquality1 + else + table.remove(diffs, pointer - 1) + pointer = pointer - 1 + end + diffs[pointer].value = bestEdit + if #bestEquality2 > 0 then + diffs[pointer + 1].value = bestEquality2 + else + table.remove(diffs, pointer + 1) + pointer = pointer - 1 + end + end + end + pointer = pointer + 1 + end end function StringDiff._bisect(text1: string, text2: string): Diffs diff --git a/plugin/src/App/Components/StringDiffVisualizer/init.lua b/plugin/src/App/Components/StringDiffVisualizer/init.lua index a826c6496..4710842ce 100644 --- a/plugin/src/App/Components/StringDiffVisualizer/init.lua +++ b/plugin/src/App/Components/StringDiffVisualizer/init.lua @@ -5,15 +5,16 @@ local Packages = Rojo.Packages local Roact = require(Packages.Roact) local Log = require(Packages.Log) local Highlighter = require(Packages.Highlighter) +Highlighter.matchStudioSettings() local StringDiff = require(script:FindFirstChild("StringDiff")) local Timer = require(Plugin.Timer) +local Assets = require(Plugin.Assets) local Theme = require(Plugin.App.Theme) local getTextBoundsAsync = require(Plugin.App.getTextBoundsAsync) -local CodeLabel = require(Plugin.App.Components.CodeLabel) local BorderedContainer = require(Plugin.App.Components.BorderedContainer) -local ScrollingFrame = require(Plugin.App.Components.ScrollingFrame) +local VirtualScroller = require(Plugin.App.Components.VirtualScroller) local e = Roact.createElement @@ -21,26 +22,37 @@ local StringDiffVisualizer = Roact.Component:extend("StringDiffVisualizer") function StringDiffVisualizer:init() self.scriptBackground, self.setScriptBackground = Roact.createBinding(Color3.fromRGB(0, 0, 0)) - self.contentSize, self.setContentSize = Roact.createBinding(Vector2.new(0, 0)) + self.updateEvent = Instance.new("BindableEvent") + self.lineHeight, self.setLineHeight = Roact.createBinding(15) + self.canvasPosition, self.setCanvasPosition = Roact.createBinding(Vector2.zero) + self.windowWidth, self.setWindowWidth = Roact.createBinding(math.huge) -- Ensure that the script background is up to date with the current theme self.themeChangedConnection = settings().Studio.ThemeChanged:Connect(function() - task.defer(function() - -- Defer to allow Highlighter to process the theme change first + task.delay(1 / 20, function() + -- Delay to allow Highlighter to process the theme change first self:updateScriptBackground() + -- Refresh the code label colors too + self.updateEvent:Fire() end) end) self:updateScriptBackground() self:setState({ - add = {}, - remove = {}, + maxLines = 0, + currentRichTextLines = {}, + incomingRichTextLines = {}, + currentDiffs = {}, + currentLineNumbers = {}, + incomingDiffs = {}, + incomingLineNumbers = {}, }) end function StringDiffVisualizer:willUnmount() self.themeChangedConnection:Disconnect() + self.updateEvent:Destroy() end function StringDiffVisualizer:updateScriptBackground() @@ -51,96 +63,286 @@ function StringDiffVisualizer:updateScriptBackground() end function StringDiffVisualizer:didUpdate(previousProps) - if previousProps.oldString ~= self.props.oldString or previousProps.newString ~= self.props.newString then - local add, remove = self:calculateDiffLines() - self:setState({ - add = add, - remove = remove, - }) + if + previousProps.currentString ~= self.props.currentString + or previousProps.incomingString ~= self.props.incomingString + then + self:updateDiffs() end end function StringDiffVisualizer:calculateContentSize(theme) - local oldString, newString = self.props.oldString, self.props.newString + local currentString, incomingString = self.props.currentString, self.props.incomingString - local oldStringBounds = getTextBoundsAsync(oldString, theme.Font.Code, theme.TextSize.Code, math.huge) - local newStringBounds = getTextBoundsAsync(newString, theme.Font.Code, theme.TextSize.Code, math.huge) + local currentStringBounds = getTextBoundsAsync(currentString, theme.Font.Code, theme.TextSize.Code, math.huge) + local incomingStringBounds = getTextBoundsAsync(incomingString, theme.Font.Code, theme.TextSize.Code, math.huge) - self.setContentSize( - Vector2.new(math.max(oldStringBounds.X, newStringBounds.X), math.max(oldStringBounds.Y, newStringBounds.Y)) + return Vector2.new( + math.max(currentStringBounds.X, incomingStringBounds.X), + math.max(currentStringBounds.Y, incomingStringBounds.Y) ) end -function StringDiffVisualizer:calculateDiffLines() - Timer.start("StringDiffVisualizer:calculateDiffLines") - local oldString, newString = self.props.oldString, self.props.newString +function StringDiffVisualizer:updateDiffs() + Timer.start("StringDiffVisualizer:updateDiffs") + local currentString, incomingString = self.props.currentString, self.props.incomingString -- Diff the two texts local startClock = os.clock() - local diffs = StringDiff.findDiffs(oldString, newString) + local diffs = + StringDiff.findDiffs((string.gsub(currentString, "\t", " ")), (string.gsub(incomingString, "\t", " "))) local stopClock = os.clock() Log.trace( "Diffing {} byte and {} byte strings took {} microseconds and found {} diff sections", - #oldString, - #newString, + #currentString, + #incomingString, math.round((stopClock - startClock) * 1000 * 1000), #diffs ) - -- Determine which lines to highlight - local add, remove = {}, {} + -- Build the rich text lines + local currentRichTextLines = Highlighter.buildRichTextLines({ + src = currentString, + }) + local incomingRichTextLines = Highlighter.buildRichTextLines({ + src = incomingString, + }) + + local maxLines = math.max(#currentRichTextLines, #incomingRichTextLines) + + -- Find the diff locations + local currentDiffs, incomingDiffs = {}, {} + local currentSpacers, incomingSpacers = {}, {} - local oldLineNum, newLineNum = 1, 1 + local firstDiffLineNum = 0 + + local currentLineNum, currentIdx, incomingLineNum, incomingIdx = 1, 0, 1, 0 for _, diff in diffs do local actionType, text = diff.actionType, diff.value - local lines = select(2, string.gsub(text, "\n", "\n")) + local lines = string.split(text, "\n") if actionType == StringDiff.ActionTypes.Equal then - oldLineNum += lines - newLineNum += lines + for i, line in lines do + if i > 1 then + currentLineNum += 1 + currentIdx = 0 + incomingLineNum += 1 + incomingIdx = 0 + end + currentIdx += #line + incomingIdx += #line + end elseif actionType == StringDiff.ActionTypes.Insert then - if lines > 0 then - local textLines = string.split(text, "\n") - for i, textLine in textLines do - if string.match(textLine, "%S") then - add[newLineNum + i - 1] = true - end + if firstDiffLineNum == 0 then + firstDiffLineNum = incomingLineNum + end + + for i, line in lines do + if i > 1 then + incomingLineNum += 1 + incomingIdx = 0 + + table.insert(currentSpacers, { currentLineNum = currentLineNum, incomingLineNum = incomingLineNum }) end - else - if string.match(text, "%S") then - add[newLineNum] = true + if not incomingDiffs[incomingLineNum] then + incomingDiffs[incomingLineNum] = { + { start = incomingIdx, stop = incomingIdx + #line }, + } + else + table.insert(incomingDiffs[incomingLineNum], { + start = incomingIdx, + stop = incomingIdx + #line, + }) end + incomingIdx += #line end - newLineNum += lines elseif actionType == StringDiff.ActionTypes.Delete then - if lines > 0 then - local textLines = string.split(text, "\n") - for i, textLine in textLines do - if string.match(textLine, "%S") then - remove[oldLineNum + i - 1] = true - end + if firstDiffLineNum == 0 then + firstDiffLineNum = currentLineNum + end + + for i, line in lines do + if i > 1 then + currentLineNum += 1 + currentIdx = 0 + + table.insert( + incomingSpacers, + { currentLineNum = currentLineNum, incomingLineNum = incomingLineNum } + ) end - else - if string.match(text, "%S") then - remove[oldLineNum] = true + if not currentDiffs[currentLineNum] then + currentDiffs[currentLineNum] = { + { start = currentIdx, stop = currentIdx + #line }, + } + else + table.insert(currentDiffs[currentLineNum], { + start = currentIdx, + stop = currentIdx + #line, + }) end + currentIdx += #line end - oldLineNum += lines else Log.warn("Unknown diff action: {} {}", actionType, text) end end + -- Filter out diffs that are just newlines being added/removed from existing non-empty lines. + -- This is done to make the diff visualization less noisy. + + local currentStringLines = string.split(currentString, "\n") + local incomingStringLines = string.split(incomingString, "\n") + + for lineNum, lineDiffs in currentDiffs do + if + (#lineDiffs > 1) -- Not just newline + or (lineDiffs[1].start ~= lineDiffs[1].stop) -- Not a newline at all + or (currentStringLines[lineNum] == "") -- Empty line, so the newline change is significant + then + continue + end + -- Just a noisy newline diff, clear it + currentDiffs[lineNum] = nil + end + + for lineNum, lineDiffs in incomingDiffs do + if + (#lineDiffs > 1) -- Not just newline + or (lineDiffs[1].start ~= lineDiffs[1].stop) -- Not a newline at all + or (incomingStringLines[lineNum] == "") -- Empty line, so the newline change is significant + then + continue + end + -- Just a noisy newline diff, clear it + incomingDiffs[lineNum] = nil + end + + -- Adjust the rich text lines and their diffs to include spacers (aka nil lines) + for spacerIdx, spacer in currentSpacers do + local spacerLineNum = spacer.currentLineNum + spacerIdx - 1 + table.insert(currentRichTextLines, spacerLineNum, 0) + -- The currentDiffs that come after this spacer need to be moved down + -- without overwriting the currentDiffs that are already there + local updatedCurrentDiffs = {} + for lineNum, lineDiffs in pairs(currentDiffs) do + if lineNum >= spacerLineNum then + updatedCurrentDiffs[lineNum + 1] = lineDiffs + else + updatedCurrentDiffs[lineNum] = lineDiffs + end + end + currentDiffs = updatedCurrentDiffs + end + for spacerIdx, spacer in incomingSpacers do + local spacerLineNum = spacer.incomingLineNum + spacerIdx - 1 + table.insert(incomingRichTextLines, spacerLineNum, 0) + + -- The incomingDiffs that come after this spacer need to be moved down + -- without overwriting the incomingDiffs that are already there + local updatedIncomingDiffs = {} + for lineNum, lineDiffs in pairs(incomingDiffs) do + if lineNum >= spacerLineNum then + updatedIncomingDiffs[lineNum + 1] = lineDiffs + else + updatedIncomingDiffs[lineNum] = lineDiffs + end + end + incomingDiffs = updatedIncomingDiffs + end + + -- Update the maxLines after we may have inserted additional lines + maxLines = math.max(#currentRichTextLines, #incomingRichTextLines) + + local currentLineNumbers, incomingLineNumbers = table.create(maxLines, 0), table.create(maxLines, 0) + local currentLineNumber, incomingLineNumber = 0, 0 + for lineNum = 1, maxLines do + if type(currentRichTextLines[lineNum]) == "string" then + currentLineNumber += 1 + currentLineNumbers[lineNum] = currentLineNumber + end + if type(incomingRichTextLines[lineNum]) == "string" then + incomingLineNumber += 1 + incomingLineNumbers[lineNum] = incomingLineNumber + end + end + Timer.stop() - return add, remove + + self:setState({ + maxLines = maxLines, + currentRichTextLines = currentRichTextLines, + incomingRichTextLines = incomingRichTextLines, + currentDiffs = currentDiffs, + currentLineNumbers = currentLineNumbers, + incomingDiffs = incomingDiffs, + incomingLineNumbers = incomingLineNumbers, + }) + + -- Scroll to the first diff line + task.defer(self.setCanvasPosition, Vector2.new(0, math.max(0, (firstDiffLineNum - 4) * 16))) end function StringDiffVisualizer:render() - local oldString, newString = self.props.oldString, self.props.newString + local currentDiffs, incomingDiffs = self.state.currentDiffs, self.state.incomingDiffs + local currentLineNumbers, incomingLineNumbers = self.state.currentLineNumbers, self.state.incomingLineNumbers + local currentRichTextLines, incomingRichTextLines = + self.state.currentRichTextLines, self.state.incomingRichTextLines + local maxLines = self.state.maxLines return Theme.with(function(theme) - self:calculateContentSize(theme) + self.setLineHeight(theme.TextSize.Code) + + -- Calculate the width of the canvas + -- (One line at a time to avoid the 200k char limit of getTextBoundsAsync) + local canvasWidth = 0 + for i = 1, maxLines do + local currentLine = currentRichTextLines[i] + if type(currentLine) == "string" and string.find(currentLine, "%S") then + local bounds = getTextBoundsAsync(currentLine, theme.Font.Code, theme.TextSize.Code, math.huge, true) + if bounds.X > canvasWidth then + canvasWidth = bounds.X + end + end + local incomingLine = incomingRichTextLines[i] + if type(incomingLine) == "string" and string.find(incomingLine, "%S") then + local bounds = getTextBoundsAsync(incomingLine, theme.Font.Code, theme.TextSize.Code, math.huge, true) + if bounds.X > canvasWidth then + canvasWidth = bounds.X + end + end + end + + local lineNumberWidth = + getTextBoundsAsync(tostring(maxLines), theme.Font.Code, theme.TextSize.Body, math.huge, true).X + + canvasWidth += lineNumberWidth + 12 + + local removalScrollMarkers = {} + local insertionScrollMarkers = {} + for lineNum in currentDiffs do + table.insert( + removalScrollMarkers, + e("Frame", { + Size = UDim2.fromScale(0.5, 1 / maxLines), + Position = UDim2.fromScale(0, (lineNum - 1) / maxLines), + BorderSizePixel = 0, + BackgroundColor3 = theme.Diff.Remove, + }) + ) + end + for lineNum in incomingDiffs do + table.insert( + insertionScrollMarkers, + e("Frame", { + Size = UDim2.fromScale(0.5, 1 / maxLines), + Position = UDim2.fromScale(0.5, (lineNum - 1) / maxLines), + BorderSizePixel = 0, + BackgroundColor3 = theme.Diff.Add, + }) + ) + end return e(BorderedContainer, { size = self.props.size, @@ -159,43 +361,201 @@ function StringDiffVisualizer:render() CornerRadius = UDim.new(0, 5), }), }), - Separator = e("Frame", { - Size = UDim2.new(0, 2, 1, 0), - Position = UDim2.new(0.5, 0, 0, 0), - AnchorPoint = Vector2.new(0.5, 0), - BorderSizePixel = 0, - BackgroundColor3 = theme.BorderedContainer.BorderColor, - BackgroundTransparency = 0.5, - }), - Old = e(ScrollingFrame, { - position = UDim2.new(0, 2, 0, 2), - size = UDim2.new(0.5, -7, 1, -4), - scrollingDirection = Enum.ScrollingDirection.XY, - transparency = self.props.transparency, - contentSize = self.contentSize, + Main = e("Frame", { + Size = UDim2.new(1, -10, 1, -2), + Position = UDim2.new(0, 2, 0, 2), + BackgroundTransparency = 1, + [Roact.Change.AbsoluteSize] = function(rbx) + self.setWindowWidth(rbx.AbsoluteSize.X * 0.5 - 10) + end, }, { - Source = e(CodeLabel, { - size = UDim2.new(1, 0, 1, 0), + Separator = e("Frame", { + Size = UDim2.new(0, 2, 1, 0), + Position = UDim2.new(0.5, 0, 0, 0), + AnchorPoint = Vector2.new(0.5, 0), + BorderSizePixel = 0, + BackgroundColor3 = theme.BorderedContainer.BorderColor, + BackgroundTransparency = 0.5, + }), + Current = e(VirtualScroller, { position = UDim2.new(0, 0, 0, 0), - text = oldString, - lineBackground = theme.Diff.Remove, - markedLines = self.state.remove, + size = UDim2.new(0.5, -1, 1, 0), + transparency = self.props.transparency, + count = maxLines, + updateEvent = self.updateEvent.Event, + canvasWidth = canvasWidth, + canvasPosition = self.canvasPosition, + onCanvasPositionChanged = self.setCanvasPosition, + render = function(i) + if type(currentRichTextLines[i]) ~= "string" then + return e("ImageLabel", { + Size = UDim2.fromScale(1, 1), + Position = UDim2.fromScale(0, 0), + BackgroundTransparency = 1, + BorderSizePixel = 0, + Image = Assets.Images.DiagonalLines, + ImageTransparency = 0.7, + ImageColor3 = theme.TextColor, + ScaleType = Enum.ScaleType.Tile, + TileSize = UDim2.new(0, 64, 4, 0), + }) + end + + local lineDiffs = currentDiffs[i] + local diffFrames = table.create(if lineDiffs then #lineDiffs else 0) + + if lineDiffs then + local charWidth = math.round(theme.TextSize.Code * 0.5) + for diffIdx, diff in lineDiffs do + local start, stop = diff.start, diff.stop + diffFrames[diffIdx] = e("Frame", { + Size = UDim2.new(0, math.max(charWidth * (stop - start), charWidth * 0.4), 1, 0), + Position = UDim2.fromOffset(charWidth * start, 0), + BackgroundColor3 = theme.Diff.Remove, + BackgroundTransparency = 0.85, + BorderSizePixel = 0, + ZIndex = -1, + }) + end + end + + return Roact.createFragment({ + LineNumber = e("TextLabel", { + Size = UDim2.new(0, lineNumberWidth + 8, 1, 0), + Text = currentLineNumbers[i] or "", + BackgroundColor3 = Color3.new(0, 0, 0), + BackgroundTransparency = 0.9, + BorderSizePixel = 0, + FontFace = theme.Font.Code, + TextSize = theme.TextSize.Body, + TextColor3 = if lineDiffs then theme.Diff.Remove else theme.SubTextColor, + TextXAlignment = Enum.TextXAlignment.Right, + }, { + Padding = e("UIPadding", { PaddingRight = UDim.new(0, 6) }), + }), + Content = e("Frame", { + Size = UDim2.new(1, -(lineNumberWidth + 10), 1, 0), + Position = UDim2.fromScale(1, 0), + AnchorPoint = Vector2.new(1, 0), + BackgroundTransparency = 1, + }, { + CodeLabel = e("TextLabel", { + Size = UDim2.fromScale(1, 1), + Position = UDim2.fromScale(0, 0), + Text = currentRichTextLines[i], + RichText = true, + BackgroundColor3 = theme.Diff.Remove, + BackgroundTransparency = if lineDiffs then 0.95 else 1, + BorderSizePixel = 0, + FontFace = theme.Font.Code, + TextSize = theme.TextSize.Code, + TextXAlignment = Enum.TextXAlignment.Left, + TextYAlignment = Enum.TextYAlignment.Top, + TextColor3 = Color3.fromRGB(255, 255, 255), + }), + DiffFrames = Roact.createFragment(diffFrames), + }), + }) + end, + getHeightBinding = function() + return self.lineHeight + end, + }), + Incoming = e(VirtualScroller, { + position = UDim2.new(0.5, 1, 0, 0), + size = UDim2.new(0.5, -1, 1, 0), + transparency = self.props.transparency, + count = maxLines, + updateEvent = self.updateEvent.Event, + canvasWidth = canvasWidth, + canvasPosition = self.canvasPosition, + onCanvasPositionChanged = self.setCanvasPosition, + render = function(i) + if type(incomingRichTextLines[i]) ~= "string" then + return e("ImageLabel", { + Size = UDim2.fromScale(1, 1), + Position = UDim2.fromScale(0, 0), + BackgroundTransparency = 1, + BorderSizePixel = 0, + Image = Assets.Images.DiagonalLines, + ImageTransparency = 0.7, + ImageColor3 = theme.TextColor, + ScaleType = Enum.ScaleType.Tile, + TileSize = UDim2.new(0, 64, 4, 0), + }) + end + + local lineDiffs = incomingDiffs[i] + local diffFrames = table.create(if lineDiffs then #lineDiffs else 0) + + if lineDiffs then + local charWidth = math.round(theme.TextSize.Code * 0.5) + for diffIdx, diff in lineDiffs do + local start, stop = diff.start, diff.stop + diffFrames[diffIdx] = e("Frame", { + Size = UDim2.new(0, math.max(charWidth * (stop - start), charWidth * 0.4), 1, 0), + Position = UDim2.fromOffset(charWidth * start, 0), + BackgroundColor3 = theme.Diff.Add, + BackgroundTransparency = 0.85, + BorderSizePixel = 0, + ZIndex = -1, + }) + end + end + + return Roact.createFragment({ + LineNumber = e("TextLabel", { + Size = UDim2.new(0, lineNumberWidth + 8, 1, 0), + Text = incomingLineNumbers[i] or "", + BackgroundColor3 = Color3.new(0, 0, 0), + BackgroundTransparency = 0.9, + BorderSizePixel = 0, + FontFace = theme.Font.Code, + TextSize = theme.TextSize.Body, + TextColor3 = if lineDiffs then theme.Diff.Add else theme.SubTextColor, + TextXAlignment = Enum.TextXAlignment.Right, + }, { + Padding = e("UIPadding", { PaddingRight = UDim.new(0, 6) }), + }), + Content = e("Frame", { + Size = UDim2.new(1, -(lineNumberWidth + 10), 1, 0), + Position = UDim2.fromScale(1, 0), + AnchorPoint = Vector2.new(1, 0), + BackgroundTransparency = 1, + }, { + CodeLabel = e("TextLabel", { + Size = UDim2.fromScale(1, 1), + Position = UDim2.fromScale(0, 0), + Text = incomingRichTextLines[i], + RichText = true, + BackgroundColor3 = theme.Diff.Add, + BackgroundTransparency = if lineDiffs then 0.95 else 1, + BorderSizePixel = 0, + FontFace = theme.Font.Code, + TextSize = theme.TextSize.Code, + TextXAlignment = Enum.TextXAlignment.Left, + TextYAlignment = Enum.TextYAlignment.Top, + TextColor3 = Color3.fromRGB(255, 255, 255), + }), + DiffFrames = Roact.createFragment(diffFrames), + }), + }) + end, + getHeightBinding = function() + return self.lineHeight + end, }), }), - New = e(ScrollingFrame, { - position = UDim2.new(0.5, 5, 0, 2), - size = UDim2.new(0.5, -7, 1, -4), - scrollingDirection = Enum.ScrollingDirection.XY, - transparency = self.props.transparency, - contentSize = self.contentSize, + ScrollMarkers = e("Frame", { + Size = self.windowWidth:map(function(windowWidth) + return UDim2.new(0, 8, 1, -4 - (if canvasWidth > windowWidth then 10 else 0)) + end), + Position = UDim2.new(1, -2, 0, 2), + AnchorPoint = Vector2.new(1, 0), + BackgroundTransparency = 1, }, { - Source = e(CodeLabel, { - size = UDim2.new(1, 0, 1, 0), - position = UDim2.new(0, 0, 0, 0), - text = newString, - lineBackground = theme.Diff.Add, - markedLines = self.state.add, - }), + insertions = Roact.createFragment(insertionScrollMarkers), + removals = Roact.createFragment(removalScrollMarkers), }), }) end) diff --git a/plugin/src/App/Components/VirtualScroller.lua b/plugin/src/App/Components/VirtualScroller.lua index 466da8a4f..63d966ca8 100644 --- a/plugin/src/App/Components/VirtualScroller.lua +++ b/plugin/src/App/Components/VirtualScroller.lua @@ -15,8 +15,10 @@ local VirtualScroller = Roact.Component:extend("VirtualScroller") function VirtualScroller:init() self.scrollFrameRef = Roact.createRef() self:setState({ - WindowSize = Vector2.new(), - CanvasPosition = Vector2.new(), + WindowSize = Vector2.zero, + CanvasPosition = if self.props.canvasPosition + then self.props.canvasPosition:getValue() or Vector2.zero + else Vector2.zero, }) self.totalCanvas, self.setTotalCanvas = Roact.createBinding(0) @@ -41,6 +43,10 @@ function VirtualScroller:didMount() local canvasPositionSignal = rbx:GetPropertyChangedSignal("CanvasPosition") self.canvasPositionChanged = canvasPositionSignal:Connect(function() + if self.props.onCanvasPositionChanged then + pcall(self.props.onCanvasPositionChanged, rbx.CanvasPosition) + end + if math.abs(rbx.CanvasPosition.Y - self.state.CanvasPosition.Y) > 5 then self:setState({ CanvasPosition = rbx.CanvasPosition }) self:refresh() @@ -134,8 +140,9 @@ function VirtualScroller:render() BackgroundColor3 = props.backgroundColor3 or theme.BorderedContainer.BackgroundColor, BorderColor3 = props.borderColor3 or theme.BorderedContainer.BorderColor, CanvasSize = self.totalCanvas:map(function(s) - return UDim2.fromOffset(0, s) + return UDim2.fromOffset(props.canvasWidth or 0, s) end), + CanvasPosition = self.props.canvasPosition, ScrollBarThickness = 9, ScrollBarImageColor3 = theme.ScrollBarColor, ScrollBarImageTransparency = props.transparency:map(function(value) @@ -146,7 +153,7 @@ function VirtualScroller:render() BottomImage = Assets.Images.ScrollBar.Bottom, ElasticBehavior = Enum.ElasticBehavior.Always, - ScrollingDirection = Enum.ScrollingDirection.Y, + ScrollingDirection = Enum.ScrollingDirection.XY, VerticalScrollBarInset = Enum.ScrollBarInset.ScrollBar, [Roact.Ref] = self.scrollFrameRef, }, { diff --git a/plugin/src/App/StatusPages/Confirming.lua b/plugin/src/App/StatusPages/Confirming.lua index 3f3958a4c..b7f65d2b7 100644 --- a/plugin/src/App/StatusPages/Confirming.lua +++ b/plugin/src/App/StatusPages/Confirming.lua @@ -26,8 +26,8 @@ function ConfirmingPage:init() self:setState({ patchTree = nil, showingStringDiff = false, - oldString = "", - newString = "", + currentString = "", + incomingString = "", showingTableDiff = false, oldTable = {}, newTable = {}, @@ -81,11 +81,11 @@ function ConfirmingPage:render() patchTree = self.state.patchTree, - showStringDiff = function(oldString: string, newString: string) + showStringDiff = function(currentString: string, incomingString: string) self:setState({ showingStringDiff = true, - oldString = oldString, - newString = newString, + currentString = currentString, + incomingString = incomingString, }) end, showTableDiff = function(oldTable: { [any]: any? }, newTable: { [any]: any? }) @@ -192,8 +192,8 @@ function ConfirmingPage:render() anchorPoint = Vector2.new(0, 0), transparency = self.props.transparency, - oldString = self.state.oldString, - newString = self.state.newString, + currentString = self.state.currentString, + incomingString = self.state.incomingString, }), }), }), diff --git a/plugin/src/App/StatusPages/Connected.lua b/plugin/src/App/StatusPages/Connected.lua index 1d089e28f..2a489366c 100644 --- a/plugin/src/App/StatusPages/Connected.lua +++ b/plugin/src/App/StatusPages/Connected.lua @@ -307,8 +307,8 @@ function ConnectedPage:init() renderChanges = false, hoveringChangeInfo = false, showingStringDiff = false, - oldString = "", - newString = "", + currentString = "", + incomingString = "", }) self.changeInfoText, self.setChangeInfoText = Roact.createBinding("") @@ -511,11 +511,11 @@ function ConnectedPage:render() patchData = self.props.patchData, patchTree = self.props.patchTree, serveSession = self.props.serveSession, - showStringDiff = function(oldString: string, newString: string) + showStringDiff = function(currentString: string, incomingString: string) self:setState({ showingStringDiff = true, - oldString = oldString, - newString = newString, + currentString = currentString, + incomingString = incomingString, }) end, showTableDiff = function(oldTable: { [any]: any? }, newTable: { [any]: any? }) @@ -566,8 +566,8 @@ function ConnectedPage:render() anchorPoint = Vector2.new(0, 0), transparency = self.props.transparency, - oldString = self.state.oldString, - newString = self.state.newString, + currentString = self.state.currentString, + incomingString = self.state.incomingString, }), }), }), diff --git a/plugin/src/Assets.lua b/plugin/src/Assets.lua index e48c1217e..73c208458 100644 --- a/plugin/src/Assets.lua +++ b/plugin/src/Assets.lua @@ -20,6 +20,7 @@ local Assets = { PluginButton = "rbxassetid://3405341609", PluginButtonConnected = "rbxassetid://9529783993", PluginButtonWarning = "rbxassetid://9529784530", + DiagonalLines = "rbxassetid://117018699617466", Icons = { Close = "rbxassetid://6012985953", Back = "rbxassetid://6017213752",