Merge pull request #166 from arr2036/mediawiki_compat

Gollum doesn't play nice with un-canonicalized filenames, and breaks offsite links when replacing mediawiki

lib/gollum/committer.rb

@@ -73,7 +73,7 @@ module Gollum
     #
     # dir    - The String subdirectory of the Gollum::Page without any
     #          prefix or suffix slashes (e.g. "foo/bar").
-    # name   - The String Gollum::Page name.
+    # name   - The String Gollum::Page filename_stripped.
     # format - The Symbol Gollum::Page format.
     # data   - The String wiki data to store in the tree map.
     # allow_same_ext - A Boolean determining if the tree map allows the same
@@ -111,7 +111,8 @@ module Gollum
     # is a working directory present.
     #
     # dir    - The String directory in which the file lives.
-    # name   - The String name of the page (may be in human format).
+    # name   - The String name of the page or the stripped filename
+    #          (should be pre-canonicalized if required).
     # format - The Symbol format of the page.
     #
     # Returns nothing.

lib/gollum/page.rb

@@ -76,11 +76,20 @@ module Gollum
     end
 
     # Reusable filter to turn a filename (without path) into a canonical name.
-    # Strips extension, converts spaces to dashes.
+    # Strips extension, converts dashes to spaces.
     #
     # Returns the filtered String.
     def self.canonicalize_filename(filename)
-      filename.split('.')[0..-2].join('.').gsub('-', ' ')
+      strip_filename(filename).gsub('-', ' ')
+    end
+
+    # Reusable filter to strip extension and path from filename
+    #
+    # filename - The string path or filename to strip
+    #
+    # Returns the stripped String. 
+    def self.strip_filename(filename)
+      ::File.basename(filename, ::File.extname(filename))
     end
 
     # Public: Initialize a page.
@@ -100,6 +109,13 @@ module Gollum
       @blob && @blob.name
     end
 
+    # Public: The on-disk filename of the page with extension stripped.
+    #
+    # Returns the String name.
+    def filename_stripped
+      self.class.strip_filename(filename)
+    end
+
     # Public: The canonical page name without extension, and dashes converted
     # to spaces.
     #
@@ -246,17 +262,22 @@ module Gollum
 
     # Convert a human page name into a canonical page name.
     #
-    # name - The String human page name.
+    # name           - The String human page name.
+    # char_white_sub - Substitution for whitespace
+    # char_other_sub - Substitution for other special chars
     #
     # Examples
     #
     #   Page.cname("Bilbo Baggins")
     #   # => 'Bilbo-Baggins'
     #
+    #   Page.cname("Bilbo Baggins",'_')
+    #   # => 'Bilbo_Baggins'
+    #
     # Returns the String canonical name.
-    def self.cname(name)
+    def self.cname(name, char_white_sub = '-', char_other_sub = '-')
-      name.respond_to?(:gsub)      ?
+      name.respond_to?(:gsub) ?
-        name.gsub(%r{[ /<>]}, '-') :
+        name.gsub(%r{\s},char_white_sub).gsub(%r{[/<>+]}, char_other_sub) :
         ''
     end
 
@@ -370,10 +391,11 @@ module Gollum
     # Returns a Boolean.
     def page_match(name, filename)
       if match = self.class.valid_filename?(filename)
-        Page.cname(name).downcase == Page.cname(match).downcase
+        @wiki.ws_subs.each do |sub|
-      else
+          return true if Page.cname(name).downcase == Page.cname(match, sub).downcase
-        false
+        end
       end
+      false
     end
 
     # Loads a sub page.  Sub page nanes (footers) are prefixed with

lib/gollum/wiki.rb

@@ -21,6 +21,9 @@ module Gollum
       # Sets the default email for commits.
       attr_accessor :default_committer_email
       
+      # Array of chars to substitute whitespace for when trying to locate file in git repo.
+      attr_accessor :default_ws_subs
+      
       # Sets sanitization options. Set to false to deactivate
       # sanitization altogether.
       attr_writer :sanitization
@@ -103,6 +106,8 @@ module Gollum
     self.default_committer_name  = 'Anonymous'
     self.default_committer_email = 'anon@anon.com'
     
+    self.default_ws_subs = ['_','-']
+
     # The String base path to prefix to internal links. For example, when set
     # to "/wiki", the page "Hobbit" will be linked as "/wiki/Hobbit". Defaults
     # to "/".
@@ -120,6 +125,9 @@ module Gollum
     # Gets the String directory in which all page files reside.
     attr_reader :page_file_dir
     
+    # Gets the Array of chars to sub for ws in filenames.
+    attr_reader :ws_subs
+
     # Public: Initialize a new Gollum Repo.
     #
     # path    - The String path to the Git repository that holds the Gollum
@@ -134,6 +142,7 @@ module Gollum
     #           :sanitization  - An instance of Sanitization.
     #           :page_file_dir - String the directory in which all page files reside
     #           :ref - String the repository ref to retrieve pages from
+    #           :ws_subs       - Array of chars to sub for ws in filenames.
     #
     # Returns a fresh Gollum::Repo.
     def initialize(path, options = {})
@@ -151,6 +160,8 @@ module Gollum
       @repo          = @access.repo
       @ref           = options[:ref] || self.class.default_ref
       @sanitization  = options[:sanitization] || self.class.sanitization
+      @ws_subs       = options[:ws_subs] ||
+        self.class.default_ws_subs
       @history_sanitization = options[:history_sanitization] ||
         self.class.history_sanitization
     end
@@ -229,11 +240,13 @@ module Gollum
         Committer.new(self, commit)
       end
       
-      committer.add_to_index('', name, format, data)
+      filename = Gollum::Page.cname(name)
+      
+      committer.add_to_index('', filename, format, data)
 
       committer.after_commit do |index, sha|
         @access.refresh
-        index.update_working_dir('', name, format)
+        index.update_working_dir('', filename, format)
       end
 
       multi_commit ? committer : committer.commit
@@ -266,6 +279,9 @@ module Gollum
       format ||= page.format
       dir      = ::File.dirname(page.path)
       dir      = '' if dir == '.'   
+      filename = (rename = page.name != name) ?
+        Gollum::Page.cname(name) : page.filename_stripped
+      
       multi_commit = false
 
       committer = if obj = commit[:committer]
@@ -275,17 +291,17 @@ module Gollum
         Committer.new(self, commit)
       end
       
-      if page.name == name && page.format == format
+      if !rename && page.format == format
         committer.add(page.path, normalize(data))
       else
         committer.delete(page.path)
-        committer.add_to_index(dir, name, format, data, :allow_same_ext)
+        committer.add_to_index(dir, filename, format, data, :allow_same_ext)
       end
       
       committer.after_commit do |index, sha|
         @access.refresh
-        index.update_working_dir(dir, page.name, page.format)
+        index.update_working_dir(dir, page.filename_stripped, page.format)
-        index.update_working_dir(dir, name, format)
+        index.update_working_dir(dir, filename, format)
       end
 
       multi_commit ? committer : committer.commit
@@ -324,7 +340,7 @@ module Gollum
         dir = '' if dir == '.'
 
         @access.refresh
-        index.update_working_dir(dir, page.name, page.format)
+        index.update_working_dir(dir, page.filename_stripped, page.format)
       end
 
       multi_commit ? committer : committer.commit
@@ -362,7 +378,7 @@ module Gollum
 
         files = []
         if page
-          files << [page.path, page.name, page.format]
+          files << [page.path, page.filename_stripped, page.format]
         else
           # Grit::Diff can't parse reverse diffs.... yet
           patch.each_line do |line|
@@ -522,13 +538,12 @@ module Gollum
 
     # Assemble a Page's filename from its name and format.
     #
-    # name   - The String name of the page (may be in human format).
+    # name   - The String name of the page (should be pre-canonicalized).
     # format - The Symbol format of the page.
     #
     # Returns the String filename.
     def page_file_name(name, format)
-      ext = @page_class.format_to_ext(format)
+      name + '.' + @page_class.format_to_ext(format)
-      @page_class.cname(name) + '.' + ext
     end
 
     # Fill an array with a list of pages.

test/examples/lotr.git/COMMIT_EDITMSG

@@ -1 +1 @@
-add sidebars
+Test out whitespace with Sam

test/examples/lotr.git/ORIG_HEAD

@@ -0,0 +1 @@
+a8ad3c09dd842a3517085bfadd37718856dee813

test/examples/lotr.git/logs/HEAD

@@ -1,2 +1,3 @@
 0000000000000000000000000000000000000000 60f12f4254f58801b9ee7db7bca5fa8aeefaa56b rick <technoweenie@gmail.com> 1291341857 -0800	clone: from /Users/rick/p/gollum/test/examples/lotr.git
 60f12f4254f58801b9ee7db7bca5fa8aeefaa56b a8ad3c09dd842a3517085bfadd37718856dee813 rick <technoweenie@gmail.com> 1291341922 -0800	commit: add sidebars
+a8ad3c09dd842a3517085bfadd37718856dee813 1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3 Arran Cudbard-Bell <a.cudbardb@freeradius.org> 1309107565 +0200	commit: Test out whitespace with Sam

test/examples/lotr.git/logs/refs/heads/master

@@ -1,2 +1,3 @@
 0000000000000000000000000000000000000000 60f12f4254f58801b9ee7db7bca5fa8aeefaa56b rick <technoweenie@gmail.com> 1291341857 -0800	clone: from /Users/rick/p/gollum/test/examples/lotr.git
 60f12f4254f58801b9ee7db7bca5fa8aeefaa56b a8ad3c09dd842a3517085bfadd37718856dee813 rick <technoweenie@gmail.com> 1291341922 -0800	commit: add sidebars
+a8ad3c09dd842a3517085bfadd37718856dee813 1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3 Arran Cudbard-Bell <a.cudbardb@freeradius.org> 1309107565 +0200	commit: Test out whitespace with Sam

test/examples/lotr.git/objects/1d/b89ebba7e2c14d93b94ff98cfa3708a4f0d4e3

@@ -0,0 +1,2 @@
+x
╔нKNд0─aж9┘В┬Q°GЦH#дЦp
'vh╔Иt■╕ К┐Ю,Ъм╞╞nК╨pS~]Z══╔`╤▒gK■└ыq┘XС└Фф]╞≤X|╣Y└┌c1Y┼╔╠┬O	┴Б$╙└чП1Ф╜цkО|┘ВC
+wyzскн|╙]^з▐║Ё,г~зЗв3═╥m┼S└GК╛5Ув;ТЪ'С╘Ш─Мp÷≈║Ш█╚б}3|Пj╬
KФWo

test/examples/lotr.git/objects/a6/59b3763b822dd97544621fd0beef162ea37b14

@@ -0,0 +1,4 @@
+x
�RMK1õ¼¿bè¥
+e÷¢ÄTPOµ·%È4;Ý6›Y’h-ôÇ;[mK¡!‡äÍLÞG²t²„›Ûë‹)¼c·æHð‚]MÓ,�ǧ p˜(@ëeí
#œ4…ÊSPä>Ð
+<v4Õ¸G³‡¾'_ñ7<A[4o*�$ÝÏÙ×QG��
ú
+¬t�x·9Òìp„²\±M,ØZUe°×ÒB\ËäÇÕJŠ�?»íkóÝ:[6&?D âi/Ì6¬n$qà8^áå‹œÚQÖs†Œ2Ó‰uö
mY±sê`ЉÁ6”6=m÷;¶èÔEGƒë!—²Œ\Q˶ÕÂ_VeÙIX‹ç˜Œ™hÓs�Jàëš}T3Y¦He“RWäó5·ÜSŘK¨‹áTœ>¯¹üOóÇï¹Ê³ShÄA

test/examples/lotr.git/refs/heads/master

@@ -1 +1 @@
-a8ad3c09dd842a3517085bfadd37718856dee813
+1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3

test/test_committer.rb

@@ -50,7 +50,7 @@ context "Wiki" do
   end
 
   test "parents with default master ref" do
-    ref = 'a8ad3c09dd842a3517085bfadd37718856dee813'
+    ref = '1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3'
     committer = Gollum::Committer.new(@wiki)
     assert_equal ref,  committer.parents.first.sha
   end

test/test_git_access.rb

@@ -18,9 +18,9 @@ context "GitAccess" do
     assert @access.ref_map.empty?
     assert @access.tree_map.empty?
     @access.tree 'master'
-    assert_equal({"master"=>"a8ad3c09dd842a3517085bfadd37718856dee813"}, @access.ref_map)
+    assert_equal({"master"=>"1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3"}, @access.ref_map)
 
-    map = @access.tree_map['a8ad3c09dd842a3517085bfadd37718856dee813']
+    map = @access.tree_map['1db89ebba7e2c14d93b94ff98cfa3708a4f0d4e3']
     assert_equal 'Bilbo-Baggins.md',        map[0].path
     assert_equal '',                        map[0].dir
     assert_equal map[0].path,               map[0].name

test/test_page.rb

@@ -26,6 +26,26 @@ context "Page" do
     assert_equal @wiki.page('Bilbo Baggins').path, @wiki.page('bilbo baggins').path
   end
   
+  test "get existing page with hyphen" do
+    assert_equal @wiki.page('Bilbo Baggins').path, @wiki.page('Bilbo-Baggins').path
+  end
+
+  test "get existing page with underscore" do
+    assert_nil @wiki.page('Bilbo_Baggins')
+  end
+  
+  test "get existing page where filename contains whitespace, with hypen" do
+    assert_equal @wiki.page('Samwise Gamgee').path, @wiki.page('Samwise-Gamgee').path
+  end
+  
+  test "get existing page where filename contains whitespace, with underscore" do
+    assert_equal @wiki.page('Samwise Gamgee').path, @wiki.page('Samwise_Gamgee').path
+  end
+  
+  test "get existing page where filename contains whitespace, with whitespace" do
+    assert_equal @wiki.page('Samwise Gamgee').path, @wiki.page('Samwise Gamgee').path
+  end
+  
   test "get nested page" do
     page = @wiki.page('Eye Of Sauron')
     assert_equal 'Mordor/Eye-Of-Sauron.md', page.path

test/test_wiki.rb

@@ -54,12 +54,12 @@ context "Wiki" do
   test "list pages" do
     pages = @wiki.pages
     assert_equal \
-      %w(Bilbo-Baggins.md Eye-Of-Sauron.md Home.textile My-Precious.md),
+      ['Bilbo-Baggins.md', 'Eye-Of-Sauron.md', 'Home.textile', 'My-Precious.md', 'Samwise Gamgee.mediawiki'],
       pages.map { |p| p.filename }.sort
   end
 
   test "counts pages" do
-    assert_equal 4, @wiki.size
+    assert_equal 5, @wiki.size
   end
 
   test "text_data" do
@@ -232,6 +232,64 @@ context "Wiki page writing" do
   end
 end
 
+context "Wiki page writing with whitespace (filename contains whitespace)" do
+  setup do
+    @path = cloned_testpath("examples/lotr.git")
+    @wiki = Gollum::Wiki.new(@path)
+  end
+  
+  test "update_page" do
+    assert_equal :mediawiki, @wiki.page("Samwise Gamgee").format
+    assert_equal "Samwise Gamgee.mediawiki", @wiki.page("Samwise Gamgee").filename
+
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, page.name, :textile, "h1. Samwise Gamgee2", commit_details)
+
+    assert_equal :textile, @wiki.page("Samwise Gamgee").format
+    assert_equal "h1. Samwise Gamgee2", @wiki.page("Samwise Gamgee").raw_data
+    assert_equal "Samwise Gamgee.textile", @wiki.page("Samwise Gamgee").filename
+  end
+  
+  test "update page with format change, verify non-canonicalization of filename,  where filename contains Whitespace" do
+    assert_equal :mediawiki, @wiki.page("Samwise Gamgee").format
+    assert_equal "Samwise Gamgee.mediawiki", @wiki.page("Samwise Gamgee").filename
+
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, page.name, :textile, "h1. Samwise Gamgee", commit_details)
+
+    assert_equal :textile, @wiki.page("Samwise Gamgee").format
+    assert_equal "h1. Samwise Gamgee", @wiki.page("Samwise Gamgee").raw_data
+    assert_equal "Samwise Gamgee.textile", @wiki.page("Samwise Gamgee").filename
+  end
+ 
+  test "update page with name change, verify canonicalization of filename, where filename contains Whitespace" do
+    assert_equal :mediawiki, @wiki.page("Samwise Gamgee").format
+    assert_equal "Samwise Gamgee.mediawiki", @wiki.page("Samwise Gamgee").filename
+
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, 'Sam Gamgee', :textile, "h1. Samwise Gamgee", commit_details)
+
+    assert_equal "h1. Samwise Gamgee", @wiki.page("Sam Gamgee").raw_data  
+    assert_equal "Sam-Gamgee.textile", @wiki.page("Sam Gamgee").filename
+  end
+  
+  test "update page with name and format change, verify canonicalization of filename, where filename contains Whitespace" do
+    assert_equal :mediawiki, @wiki.page("Samwise Gamgee").format
+    assert_equal "Samwise Gamgee.mediawiki", @wiki.page("Samwise Gamgee").filename
+
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, 'Sam Gamgee', :textile, "h1. Samwise Gamgee", commit_details)
+
+    assert_equal :textile, @wiki.page("Sam Gamgee").format
+    assert_equal "h1. Samwise Gamgee", @wiki.page("Sam Gamgee").raw_data
+    assert_equal "Sam-Gamgee.textile", @wiki.page("Sam Gamgee").filename
+  end  
+  
+  teardown do
+    FileUtils.rm_rf(@path)
+  end
+end
+
 context "Wiki sync with working directory" do
   setup do
     @path = testpath('examples/wdtest')
@@ -287,6 +345,49 @@ context "Wiki sync with working directory" do
   end
 end
 
+context "Wiki sync with working directory (filename contains whitespace)" do
+  setup do
+    @path = cloned_testpath("examples/lotr.git")
+    @wiki = Gollum::Wiki.new(@path)
+  end
+  test "update a page with same name and format" do
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, page.name, page.format, "What we need is a few good taters.", commit_details)
+    assert_equal "What we need is a few good taters.", File.read(File.join(@path, "Samwise Gamgee.mediawiki"))
+  end
+
+  test "update a page with different name and same format" do
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, "Sam Gamgee", page.format, "What we need is a few good taters.", commit_details)
+    assert_equal "What we need is a few good taters.", File.read(File.join(@path, "Sam-Gamgee.mediawiki"))
+    assert !File.exist?(File.join(@path, "Samwise Gamgee"))
+  end
+
+  test "update a page with same name and different format" do
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, page.name, :textile, "What we need is a few good taters.", commit_details)
+    assert_equal "What we need is a few good taters.", File.read(File.join(@path, "Samwise Gamgee.textile"))
+    assert !File.exist?(File.join(@path, "Samwise Gamgee.mediawiki"))
+  end
+
+  test "update a page with different name and different format" do
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.update_page(page, "Sam Gamgee", :textile, "What we need is a few good taters.", commit_details)
+    assert_equal "What we need is a few good taters.", File.read(File.join(@path, "Sam-Gamgee.textile"))
+    assert !File.exist?(File.join(@path, "Samwise Gamgee.mediawiki"))
+  end
+
+  test "delete a page" do
+    page = @wiki.page("Samwise Gamgee")
+    @wiki.delete_page(page, commit_details)
+    assert !File.exist?(File.join(@path, "Samwise Gamgee.mediawiki"))
+  end
+  
+  teardown do
+    FileUtils.rm_r(@path)
+  end
+end
+
 context "page_file_dir option" do
   setup do
     @path = cloned_testpath('examples/page_file_dir')