Use Diffy as DiffEngine implementation

trindade · Mar 16, 2013 · 204dcc4 · 204dcc4
1 parent 19860bd
commit 204dcc4
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 9 deletions.
diff --git a/Gemfile b/Gemfile
@@ -54,6 +54,7 @@ gem 'sinatra', require: nil
 gem 'slim'  # required for sidekiq-web
 gem 'therubyracer', require: 'v8'
 gem 'thin'
+gem 'diffy'
 
 # Gem that enables support for plugins. It is required.
 gem 'discourse_plugin', path: 'vendor/gems/discourse_plugin'

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -157,6 +157,7 @@ GEM
     daemons (1.1.9)
     debug_inspector (0.0.2)
     diff-lcs (1.1.3)
+    diffy (2.1.3)
     em-redis (0.3.0)
       eventmachine
     erubis (2.7.0)
@@ -475,6 +476,7 @@ DEPENDENCIES
   binding_of_caller
   certified
   clockwork
+  diffy
   discourse_emoji!
   discourse_plugin!
   em-redis

diff --git a/docs/DEVELOPER-ADVANCED.md b/docs/DEVELOPER-ADVANCED.md
@@ -17,11 +17,12 @@ to rails, you are likely much better off with our **[Discourse Vagrant Developer
 ## Before you start Rails
 
 1. `bundle install`
-2. `rake db:migrate`
-3. `rake db:test:prepare`
-4. `rake db:seed_fu`
-5. Try running the specs: `bundle exec rspec`
-6. `bundle exec rails server`
+2. `rake db:create`
+3. `rake db:migrate`
+4. `rake db:test:prepare`
+5. `rake db:seed_fu`
+6. Try running the specs: `bundle exec rspec`
+7. `bundle exec rails server`
 
 You should now be able to connect to rails on http://localhost:3000 - try it out! The seed data includes a pinned topic that explains how to get an admin account, so start there! Happy hacking!
 

diff --git a/lib/diff_engine.rb b/lib/diff_engine.rb
@@ -1,21 +1,24 @@
 # This class is used to generate diffs, it will be consumed by the UI on
 # on the client the displays diffs.
 #
-# Ruby has the diff/lcs engine that can do some of the work, the devil
-#  is in the details
+# There are potential performance issues associated with diffing large amounts of completely
+# different text, see answer here for optimization if needed
+# http://meta.stackoverflow.com/questions/127497/suggested-edit-diff-shows-different-results-depending-upon-mode
 
 class DiffEngine
 
-  # generate an html friendly diff similar to the way Stack Exchange generate
-  #  html diffs
+  # generate an html friendly diff similar to the way Stack Exchange generates
+  # html diffs
   #
   #  returns: html containing decorations indicating the changes
   def self.html_diff(html_before, html_after)
+    Diffy::Diff.new(html_before, html_after).to_s(:html)
   end
 
   # same as html diff, except that it operates on markdown
   #
   # returns html containing decorated areas where diff happened
   def self.markdown_diff(markdown_before, markdown_after)
+    Diffy::Diff.new(markdown_before, markdown_after).to_s(:html)
   end
 end
diff --git a/spec/components/diff_engine_spec.rb b/spec/components/diff_engine_spec.rb
@@ -0,0 +1,58 @@
+require 'spec_helper'
+require 'diff_engine'
+
+describe DiffEngine do
+
+  let(:html_before) do
+    <<-HTML.strip_heredoc
+    <context>
+    <original>text</original>
+    </context>
+    HTML
+  end
+
+  let(:markdown_special_characters) do
+    "=\`*_{}[]()#+-.!"
+  end
+
+  it "escapes input html to markup with diff html" do
+    diff = DiffEngine.html_diff("<html>", "")
+
+    diff.should include("&lt;html&gt;")
+  end
+
+  it "generates an html diff with ins and dels for changed" do
+    html_after = html_before
+      .gsub(/original/, "changed")
+
+    diff = DiffEngine.html_diff(html_before, html_after)
+
+    diff.should match(/del.*?original.*?del/)
+    diff.should match(/ins.*?changed.*?ins/)
+  end
+
+  it "generates an html diff with only ins for inserted" do
+    html_after = "#{html_before}\nnew"
+
+    diff = DiffEngine.html_diff(html_before, html_after)
+
+    diff.should include("ins")
+    diff.should_not include("del")
+  end
+
+  it "generates an html diff with only unchanged for unchanged" do
+    html_after = html_before
+
+    diff = DiffEngine.html_diff(html_before, html_after)
+
+    diff.should include("unchanged")
+    diff.should_not include("del", "ins")
+  end
+
+  it "handles markdown special characters" do
+    diff = DiffEngine.markdown_diff(markdown_special_characters, "")
+
+    diff.should include(markdown_special_characters)
+  end
+
+end