Skip to content

Commit

Permalink
Fix cost for ReplacedComment
Browse files Browse the repository at this point in the history
This needs to be 2x novel nodes, or we prefer it far too often.
  • Loading branch information
Wilfred committed Jul 3, 2023
1 parent 3730580 commit c405b58
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 3 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

Updated Scala parser.

### Diffing

Fixed an issue with the cost model for comment replacement, leading
difftastic to prefer modified comments even when exact comment matches
are possible.

### Display

Improved word highlighting in comments when they contain numbers.
Expand Down
1 change: 1 addition & 0 deletions sample_files/comma_and_comment_after.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
funName(1 /* kinda like bar */ , /* foo */)
1 change: 1 addition & 0 deletions sample_files/comma_and_comment_before.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
funName(1 /* foo */ , /* bar */)
3 changes: 3 additions & 0 deletions sample_files/compare.expected
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ b2cace14b01c272217eec27d16adddbe -
sample_files/clojure_before.clj sample_files/clojure_after.clj
b8e17b8eb649ba0b8d29b57a23e4ac81 -

sample_files/comma_and_comment_before.js sample_files/comma_and_comment_after.js
78be73c9caf82b3aeee81d8b8e7cf3c6 -

sample_files/comma_before.js sample_files/comma_after.js
e615c73ff860651f7749dc13cb3f110f -

Expand Down
9 changes: 6 additions & 3 deletions src/diff/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,6 @@ impl Edge {
// Matching an outer delimiter is good.
EnterUnchangedDelimiter { depth_difference } => 100 + min(40, depth_difference),

// Replacing a comment is better than treating it as novel.
ReplacedComment { levenshtein_pct } => 150 + u32::from(100 - levenshtein_pct),

// Otherwise, we've added/removed a node.
NovelAtomLHS {
contiguous,
Expand Down Expand Up @@ -348,6 +345,12 @@ impl Edge {
}
cost
}
// Replacing a comment is better than treating it as
// novel. However, since ReplacedComment is an alternative
// to NovelAtomLHS nad NovelAtomRHS, we need to be
// slightly less than 2 * (300 + NOT_CONTIGUOUS_PENALTY),
// so less than 700.
ReplacedComment { levenshtein_pct } => 600 + u32::from(100 - levenshtein_pct),
}
}
}
Expand Down

0 comments on commit c405b58

Please sign in to comment.