Skip to content

Fix unified view to show both old and new text for changed lines with syntax highlighting #4

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.

Sign up for GitHub

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

Merged
JahidHasanCO merged 4 commits into from
Jul 24, 2025
Merged

Fix unified view to show both old and new text for changed lines with syntax highlighting #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4bf6625
Initial plan
Copilot Jul 24, 2025
ba1b228
Initial analysis: Identify issue with unified view syntax highlighting
Copilot Jul 24, 2025
3bfce34
Fix unified view to show both old and new lines for CHANGED entries
Copilot Jul 24, 2025
470d75e
Add test for unified view logic and verify the fix works correctly
Copilot Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
247 changes: 180 additions & 67 deletions app/src/main/java/dev/jahidhasanco/diffly/presentation/component/UnifiedCharDiffText.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,71 +48,182 @@
}

items(diffResult) { entry ->
val prefix = when (entry.type) {
DiffType.ADDED -> "+"
DiffType.DELETED -> "-"
DiffType.UNCHANGED -> " "
DiffType.CHANGED -> "~"
}
// For CHANGED entries, show both old and new lines
if (entry.type == DiffType.CHANGED) {
// Show old line first with deletion styling
entry.oldLine?.let { oldLine ->
Box(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 2.dp, vertical = 1.dp)
.background(delete.copy(alpha = 0.1f))
) {
Row(
verticalAlignment = Alignment.Top
) {
// Old line number
Text(
text = oldLineNumber.toString(),
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray
)

val line = when (entry.type) {
DiffType.ADDED -> entry.newLine
DiffType.DELETED -> entry.oldLine
DiffType.UNCHANGED -> entry.oldLine
DiffType.CHANGED -> entry.newLine
}
// New line number (empty for old line)
Text(
text = "",
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray
)

val color = when (entry.type) {
DiffType.ADDED -> added.copy(alpha = 0.1f)
DiffType.DELETED -> delete.copy(alpha = 0.1f)
DiffType.CHANGED -> delete.copy(alpha = 0.1f)
else -> Color.Transparent
}
// Prefix for deletion
Text(
text = "-",
modifier = Modifier.padding(end = 4.dp),

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
color = Color.Gray
)

// Old line content with syntax highlighting
if (isSyntaxHighlightEnabled) {
val syntaxAnnotatedString =
remember(oldLine, language, theme) {
parseCodeAsAnnotatedString(
parser, theme, language, oldLine
)
}
Text(syntaxAnnotatedString)
} else {
Text(oldLine)
}
}
}
}

line?.let {
Box(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 2.dp, vertical = 1.dp)
.background(color)
) {
Row(
verticalAlignment = Alignment.Top
// Show new line second with addition styling
entry.newLine?.let { newLine ->
Box(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 2.dp, vertical = 1.dp)
.background(added.copy(alpha = 0.1f))
) {
// Old line number
Text(text = entry.oldLine?.let { oldLineNumber.toString() }
?: "",
modifier = Modifier
.width(40.dp)
.padding(end = 2.dp),
color = Color.Gray)

// New line number
Text(text = entry.newLine?.let { newLineNumber.toString() }
?: "",
modifier = Modifier
.width(40.dp)
.padding(end = 2.dp),
color = Color.Gray)

// Prefix
Text(
text = prefix,
modifier = Modifier.padding(end = 4.dp),
color = Color.Gray
)

// Line content or char-level diff
if (entry.type == DiffType.CHANGED && !entry.charDiffs.isNullOrEmpty()) {
InlineCharDiffText(
isSyntaxHighlightEnabled,
it,
charDiffs = entry.charDiffs,
language,
parser,
theme
Row(
verticalAlignment = Alignment.Top
) {
// Old line number (empty for new line)
Text(
text = "",
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray
)
} else {

// New line number
Text(
text = newLineNumber.toString(),
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray
)

// Prefix for addition
Text(
text = "+",
modifier = Modifier.padding(end = 4.dp),

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
color = Color.Gray
)

// New line content with char-level diff or syntax highlighting
if (!entry.charDiffs.isNullOrEmpty()) {
InlineCharDiffText(
isSyntaxHighlightEnabled,
newLine,
charDiffs = entry.charDiffs,
language,
parser,
theme
)
} else {
if (isSyntaxHighlightEnabled) {
val syntaxAnnotatedString =
remember(newLine, language, theme) {
parseCodeAsAnnotatedString(
parser, theme, language, newLine
)
}
Text(syntaxAnnotatedString)
} else {
Text(newLine)
}
}
}
}
}

// Update line numbers for CHANGED
oldLineNumber++
newLineNumber++
} else {
// Handle other diff types as before
val prefix = when (entry.type) {
DiffType.ADDED -> "+"
DiffType.DELETED -> "-"
DiffType.UNCHANGED -> " "
else -> " " // This shouldn't happen now
}

val line = when (entry.type) {
DiffType.ADDED -> entry.newLine
DiffType.DELETED -> entry.oldLine
DiffType.UNCHANGED -> entry.oldLine
else -> null // This shouldn't happen now
}

val color = when (entry.type) {
DiffType.ADDED -> added.copy(alpha = 0.1f)
DiffType.DELETED -> delete.copy(alpha = 0.1f)
else -> Color.Transparent
}

line?.let {
Box(
modifier = Modifier
.fillMaxWidth()
.padding(horizontal = 2.dp, vertical = 1.dp)
.background(color)
) {
Row(
verticalAlignment = Alignment.Top
) {
// Old line number
Text(text = entry.oldLine?.let { oldLineNumber.toString() }
?: "",
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray)

// New line number
Text(text = entry.newLine?.let { newLineNumber.toString() }
?: "",
modifier = Modifier
.width(40.dp)

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
.padding(end = 2.dp),
color = Color.Gray)

// Prefix
Text(
text = prefix,
modifier = Modifier.padding(end = 4.dp),

Check warning

Code scanning / detekt

Report magic numbers. Magic number is a numeric literal that is not defined as a constant and hence it's unclear what the purpose of this number is. It's better to declare such numbers as constants and give them a proper name. By default, -1, 0, 1, and 2 are not considered to be magic numbers. Warning

This expression contains a magic number. Consider defining it to a well named constant.
color = Color.Gray
)

// Line content with syntax highlighting
if (isSyntaxHighlightEnabled) {
val syntaxAnnotatedString =
remember(line, language, theme) {
Expand All @@ -127,14 +238,16 @@
}
}
}
}

when (entry.type) {
DiffType.ADDED -> newLineNumber++
DiffType.DELETED -> oldLineNumber++
DiffType.UNCHANGED, DiffType.CHANGED -> {
oldLineNumber++
newLineNumber++
// Update line numbers for other types
when (entry.type) {
DiffType.ADDED -> newLineNumber++
DiffType.DELETED -> oldLineNumber++
DiffType.UNCHANGED -> {
oldLineNumber++
newLineNumber++
}
else -> {} // CHANGED is handled above
}
}
}
Expand Down
108 changes: 108 additions & 0 deletions app/src/test/java/dev/jahidhasanco/diffly/presentation/component/UnifiedCharDiffTextTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
package dev.jahidhasanco.diffly.presentation.component

Check warning

Code scanning / detekt

Checks whether files end with a line separator. Warning test

The file /home/runner/work/diffly/diffly/app/src/test/java/dev/jahidhasanco/diffly/presentation/component/UnifiedCharDiffTextTest.kt is not ending with a new line.

import dev.jahidhasanco.diffly.domain.model.DiffEntry
import dev.jahidhasanco.diffly.domain.model.DiffType
import dev.jahidhasanco.diffly.domain.model.CharDiff
import dev.jahidhasanco.diffly.domain.model.CharDiffType
import kotlin.test.Test

/**
* Unit test to verify UnifiedCharDiffText logic for CHANGED entries
*/
class UnifiedCharDiffTextTest {

@Test
fun testUnifiedViewLogicWithChangedEntries() {

Check warning

Code scanning / detekt

One method should have one responsibility. Long methods tend to handle many things at once. Prefer smaller methods to make them easier to understand. Warning test

The function testUnifiedViewLogicWithChangedEntries is too long (78). The maximum length is 60.
// Test data that represents different types of diffs
val diffResult = listOf(
DiffEntry("line 1", "line 1", DiffType.UNCHANGED),
DiffEntry("old line 2", "new line 2", DiffType.CHANGED, listOf(
CharDiff('o', CharDiffType.DELETED),
CharDiff('l', CharDiffType.DELETED),
CharDiff('d', CharDiffType.DELETED),
CharDiff('n', CharDiffType.INSERTED),
CharDiff('e', CharDiffType.INSERTED),
CharDiff('w', CharDiffType.INSERTED),
CharDiff(' ', CharDiffType.UNCHANGED),
CharDiff('l', CharDiffType.UNCHANGED),
CharDiff('i', CharDiffType.UNCHANGED),
CharDiff('n', CharDiffType.UNCHANGED),
CharDiff('e', CharDiffType.UNCHANGED),
CharDiff(' ', CharDiffType.UNCHANGED),
CharDiff('2', CharDiffType.UNCHANGED)
)),
DiffEntry(null, "added line", DiffType.ADDED),
DiffEntry("deleted line", null, DiffType.DELETED),
DiffEntry("line 5", "line 5", DiffType.UNCHANGED)
)

// Simulate the line numbering logic from UnifiedCharDiffText
var oldLineNumber = 1
var newLineNumber = 1
val renderedLines = mutableListOf<String>()

for (entry in diffResult) {
when (entry.type) {
DiffType.CHANGED -> {
// Should render old line first
entry.oldLine?.let { oldLine ->
renderedLines.add("${oldLineNumber.toString().padStart(3)} ${" ".repeat(3)} - $oldLine")
}

// Should render new line second
entry.newLine?.let { newLine ->
renderedLines.add("${" ".repeat(3)} ${newLineNumber.toString().padStart(3)} + $newLine")
}

oldLineNumber++
newLineNumber++
}
DiffType.ADDED -> {
val line = entry.newLine ?: ""
renderedLines.add("${" ".repeat(3)} ${newLineNumber.toString().padStart(3)} + $line")
newLineNumber++
}
DiffType.DELETED -> {
val line = entry.oldLine ?: ""
renderedLines.add("${oldLineNumber.toString().padStart(3)} ${" ".repeat(3)} - $line")
oldLineNumber++
}
DiffType.UNCHANGED -> {
val line = entry.oldLine ?: ""
renderedLines.add("${oldLineNumber.toString().padStart(3)} ${newLineNumber.toString().padStart(3)} $line")

Check warning

Code scanning / detekt

Line detected that is longer than the defined maximum line length in the code style. Warning test

Line detected that is longer than the defined maximum line length in the code style.
oldLineNumber++
newLineNumber++
}
}
}

// Expected output for the unified view
val expected = listOf(
" 1 1 line 1",
" 2 - old line 2",
" 2 + new line 2",
" 3 + added line",
" 3 - deleted line",
" 4 4 line 5"
)

println("=== Unified View Test Results ===")
println("Expected:")
expected.forEach { println(" $it") }
println("\nActual:")
renderedLines.forEach { println(" $it") }

// Verify the results
assert(renderedLines.size == expected.size) {
"Expected ${expected.size} lines but got ${renderedLines.size}"
}

for (i in expected.indices) {
assert(renderedLines[i] == expected[i]) {
"Line $i mismatch:\nExpected: '${expected[i]}'\nActual: '${renderedLines[i]}'"
}
}

println("\n✅ Test passed! Unified view correctly shows both old and new lines for CHANGED entries.")
}
}
Empty file modified gradlew
100644 → 100755
View file
Open in desktop
Empty file.
Loading