Further tweaking of markup error handling code

app/controllers/admin_controller.rb

@@ -6,19 +6,17 @@ class AdminController < ApplicationController
 
   def create_system
     if @wiki.setup?
-      flash[:error] = <<-EOL
-          Wiki has already been created in '#{@wiki.storage_path}'. Shut down Instiki and delete 
-          this directory if you want to recreate it from scratch.<br/><br/>
-          (WARNING: this will destroy content of your current wiki).
-      EOL
+      flash[:error] = 
+          "Wiki has already been created in '#{@wiki.storage_path}'. " +
+          "Shut down Instiki and delete this directory if you want to recreate it from scratch." +
+          "\n\n" +
+          "(WARNING: this will destroy content of your current wiki)."
       redirect_home(@wiki.webs.keys.first)
     elsif @params['web_name']
       # form submitted -> create a wiki
       @wiki.setup(@params['password'], @params['web_name'], @params['web_address']) 
-      flash[:info] = <<-EOL
-          Your new wiki '#{@params['web_name']}' is created!<br/>
-          Please edit its home page and press Submit when finished.
-      EOL
+      flash[:info] = "Your new wiki '#{@params['web_name']}' is created!\n" + 
+          "Please edit its home page and press Submit when finished."
       redirect_to :web => @params['web_address'], :controller => 'wiki', :action => 'new', 
           :id => 'HomePage'
     else

app/controllers/wiki_controller.rb

@@ -122,7 +122,7 @@ class WikiController < ApplicationController
 
   def edit
     if @page.nil?
-      redirect_to :action => 'index'
+      redirect_home
     elsif @page.locked?(Time.now) and not @params['break_lock']
       redirect_to :web => @web_name, :action => 'locked', :id => @page_name
     else
@@ -172,28 +172,28 @@ class WikiController < ApplicationController
   end
 
   def save
-    redirect_to :action => 'index' if @page_name.nil?
+    redirect_home if @page_name.nil?
     cookies['author'] = @params['author']
 
     begin
-      page = @web.pages[@page_name]
-      if @web.pages[@page_name]
-        wiki.revise_page(
-            @web_name, @page_name, @params['content'], Time.now, 
-            Author.new(@params['author'], remote_ip)
-        )
-        page.unlock
+      if @page
+        wiki.revise_page(@web_name, @page_name, @params['content'], Time.now, 
+            Author.new(@params['author'], remote_ip))
+        @page.unlock
       else
-        wiki.write_page(
-            @web_name, @page_name, @params['content'], Time.now, 
-            Author.new(@params['author'], remote_ip)
-        )
+        wiki.write_page(@web_name, @page_name, @params['content'], Time.now, 
+            Author.new(@params['author'], remote_ip))
       end
       redirect_to_page @page_name
     rescue => e
-      page.unlock if defined? page
       flash[:error] = e
-      redirect_to :action => 'edit', :web => @web_name, :id => @page_name
+      flash[:content] = @params['content']
+      if @page
+        @page.unlock
+        redirect_to :action => 'edit', :web => @web_name, :id => @page_name
+      else
+        redirect_to :action => 'new', :web => @web_name, :id => @page_name
+      end
     end
   end
 

app/helpers/application_helper.rb

@@ -41,7 +41,6 @@ module ApplicationHelper
         html_options)
   end
   
-  
   # Creates a hyperlink to a Wiki page, or to a "new page" form if the page doesn't exist yet
   def link_to_page(page_name, web = @web, text = nil, options = {})
     raise 'Web not defined' if web.nil?
@@ -65,4 +64,9 @@ module ApplicationHelper
     end
   end
 
+  # Performs HTML escaping on text, but keeps linefeeds intact (by replacing them with <br/>)
+  def escape_preserving_linefeeds(text)
+    h(text).gsub(/\n/, '<br/>')
+  end
+
 end

app/models/page.rb

@@ -23,6 +23,10 @@ class Page
           "You have tried to save page '#{name}' without changing its content")
     end
 
+    # Try to render content to make sure that markup engine can take it,
+    # before addin a revision to the page
+    Revision.new(self, @revisions.length, content, created_at, author).force_rendering
+
     # A user may change a page, look at it and make some more changes - several times.
     # Not to record every such iteration as a new revision, if the previous revision was done 
     # by the same author, not more than 30 minutes ago, then update the last revision instead of

app/models/revision.rb

@@ -26,6 +26,8 @@ class Revision
     ).strftime "%B %e, %Y %H:%M" 
   end
 
+
+# todo: drop next_revision, previuous_revision and number from here - unused code
   def next_revision
     page.revisions[number + 1]
   end
@@ -107,15 +109,17 @@ class Revision
   def force_rendering
     begin
       display_content.render!
-    rescue Exception => e
+    rescue => e
       ApplicationController.logger.error "Failed rendering page #{@name}"
       ApplicationController.logger.error e
-      message = e.message.gsub(/\n/, '<br/>')
+      message = e.message
       # substitute content with an error message
-      content = <<-EOL
+      self.content = <<-EOL
           <p>Markup engine has failed to render this page, raising the following error:</p>
           <p>#{message}</p>
+          <pre>#{self.content}</pre>
       EOL
+      clear_display_cache
       raise e
     end
   end

app/models/web.rb

@@ -46,8 +46,8 @@ class Web
 
    def add_page(name, content, created_at, author)
      page = Page.new(self, name)
-     @pages[page.name] = page
      page.revise(content, created_at, author)
+     @pages[page.name] = page
    end
   
   def address=(the_address)

app/models/wiki_service.rb

@@ -38,6 +38,34 @@ module AbstractWikiService
     @system = {}
   end
   
+  def edit_web(old_address, new_address, name, markup, color, additional_style, safe_mode = false, 
+      password = nil, published = false, brackets_only = false, count_pages = false, 
+      allow_uploads = true, max_upload_size = nil)
+
+    if not @webs.key? old_address
+      raise Instiki::ValidationError.new("Web with address '#{old_address}' does not exist")
+    end
+
+    if old_address != new_address
+      if @webs.key? new_address
+        raise Instiki::ValidationError.new("There is already a web with address '#{new_address}'")
+      end
+      @webs[new_address] = @webs[old_address]
+      @webs.delete(old_address)
+      @webs[new_address].address = new_address
+    end
+
+    web = @webs[new_address]
+    web.refresh_revisions if settings_changed?(web, markup, safe_mode, brackets_only)
+
+    web.name, web.markup, web.color, web.additional_style, web.safe_mode = 
+      name, markup, color, additional_style, safe_mode
+
+    web.password, web.published, web.brackets_only, web.count_pages =
+      password, published, brackets_only, count_pages, allow_uploads
+    web.allow_uploads, web.max_upload_size = allow_uploads, max_upload_size.to_i
+  end
+
   def read_page(web_address, page_name)
     ApplicationController.logger.debug "Reading page '#{page_name}' from web '#{web_address}'"
     web = @webs[web_address]
@@ -74,42 +102,14 @@ module AbstractWikiService
     not (@webs.empty?)
   end
 
-  def edit_web(old_address, new_address, name, markup, color, additional_style, safe_mode = false, 
-      password = nil, published = false, brackets_only = false, count_pages = false, 
-      allow_uploads = true, max_upload_size = nil)
-
-    if not @webs.key? old_address
-      raise Instiki::ValidationError.new("Web with address '#{old_address}' does not exist")
-    end
-
-    if old_address != new_address
-      if @webs.key? new_address
-        raise Instiki::ValidationError.new("There is already a web with address '#{new_address}'")
-      end
-      @webs[new_address] = @webs[old_address]
-      @webs.delete(old_address)
-      @webs[new_address].address = new_address
-    end
-
-    web = @webs[new_address]
-    web.refresh_revisions if settings_changed?(web, markup, safe_mode, brackets_only)
-
-    web.name, web.markup, web.color, web.additional_style, web.safe_mode = 
-      name, markup, color, additional_style, safe_mode
-
-    web.password, web.published, web.brackets_only, web.count_pages =
-      password, published, brackets_only, count_pages, allow_uploads
-    web.allow_uploads, web.max_upload_size = allow_uploads, max_upload_size.to_i
+  def storage_path
+    self.class.storage_path
   end
   
   def write_page(web_address, page_name, content, written_on, author)
     @webs[web_address].add_page(page_name, content, written_on, author)
   end
 
-  def storage_path
-    self.class.storage_path
-  end
-  
   private
     def settings_changed?(web, markup, safe_mode, brackets_only)
       web.markup != markup || 

app/views/layouts/default.rhtml

@@ -61,11 +61,11 @@ PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
   </h1>
 
 <% if @error or @flash[:error] %> <div id="error">
-    <hr/><p><%= h(@error || @flash[:error]) %></p><hr/></div>
+    <hr/><p><%= escape_preserving_linefeeds(@error || @flash[:error]) %></p><hr/></div>
 <% end %>
 
 <% if @flash[:info] %> <div id="info">
-    <hr/><p><%= h @flash[:info] %></p><hr/></div>
+    <hr/><p><%= escape_preserving_linefeeds @flash[:info] %></p><hr/></div>
 <% end %>
 
 <%= render 'navigation' unless @web.nil? || @hide_navigation %>

app/views/wiki/edit.rhtml

@@ -4,8 +4,6 @@
   @hide_navigation = true
 %>
 
-<%= "<p style='color:red'>Please correct the error that caused this error in rendering:<br/><small>#{@params["msg"]}</small></p>" if @params["msg"] %>
-
 <div id="MarkupHelp" style="float: right; width: 250px; margin-top: 5px">
   <%= render("#{@web.markup}_help") %>
   <%= render 'wiki_words_help' %>
@@ -16,7 +14,7 @@
 %>
 
 <p>
-    <textarea name="content" style="width: 450px; height: 500px"><%= h @page.content %></textarea>
+    <textarea name="content" style="width: 450px; height: 500px"><%= h(@flash[:content] || @page.content) %></textarea>
 </p>
 <p>
     <input type="submit" value="Submit" accesskey="s"/> as 

app/views/wiki/new.rhtml

@@ -14,7 +14,7 @@
 %>
 
 <p>
-    <textarea name="content" style="width: 450px; height: 500px"></textarea>
+    <textarea name="content" style="width: 450px; height: 500px"><%= h(@flash[:content] || '') %></textarea>
 </p>
 <p>
     <input type="submit" value="Submit" accesskey="s"/> as 

test/functional/wiki_controller_test.rb

@@ -89,7 +89,8 @@ class WikiControllerTest < Test::Unit::TestCase
 
   def test_edit_unknown_page
     process 'edit', 'web' => 'wiki1', 'id' => 'UnknownPage', 'break_lock' => 'y'
-    assert_redirected_to :action => 'index'
+    assert_redirected_to :controller => 'wiki', :action => 'show', :web => 'wiki1', 
+        :id => 'HomePage'
   end
   
   def test_edit_page_with_special_symbols