Merge pull request #598 from dekimsey/new-page-in-current-directory

New page button now respects current directory

lib/gollum/frontend/app.rb

@@ -143,25 +143,57 @@ module Precious
       end
     end
 
+    post '/rename/*' do
+      wikip     = wiki_page(params[:splat].first)
+      halt 500 if wikip.nil?
+      wiki      = wikip.wiki
+      page      = wiki.paged(wikip.name, wikip.path, exact = true)
+      rename    = params[:rename]
+      halt 500 if page.nil?
+      halt 500 if rename.nil? or rename.empty?
+
+      # Fixup the rename if it is a relative path
+      # In 1.8.7 rename[0] != rename[0..0]
+      if rename[0..0] != '/'
+        source_dir = ::File.dirname(page.path)
+        source_dir = '' if source_dir == '.'
+        (target_dir, target_name) = ::File.split(rename)
+        target_dir = target_dir == '' ? source_dir : "#{source_dir}/#{target_dir}"
+        rename = "#{target_dir}/#{target_name}"
+      end
+
+      committer = Gollum::Committer.new(wiki, commit_message)
+      commit    = {:committer => committer}
+
+      success = wiki.rename_page(page, rename, commit)
+      if !success
+          # This occurs on NOOPs, for example renaming A => A
+          redirect to("/#{page.escaped_url_path}")
+          return
+      end
+      committer.commit
+
+      wikip = wiki_page(rename)
+      page = wiki.paged(wikip.name, wikip.path, exact = true)
+      return if page.nil?
+      redirect to("/#{page.escaped_url_path}")
+    end
+
     post '/edit/*' do
       path      = '/' + clean_url(sanitize_empty_params(params[:path])).to_s
       page_name = CGI.unescape(params[:page])
       wiki      = wiki_new
       page      = wiki.paged(page_name, path, exact = true)
       return if page.nil?
-      rename    = params[:rename].to_url if params[:rename]
-      name      = rename || page.name
       committer = Gollum::Committer.new(wiki, commit_message)
       commit    = {:committer => committer}
 
-      update_wiki_page(wiki, page, params[:content], commit, name, params[:format])
+      update_wiki_page(wiki, page, params[:content], commit, page.name, params[:format])
       update_wiki_page(wiki, page.header,  params[:header],  commit) if params[:header]
       update_wiki_page(wiki, page.footer,  params[:footer],  commit) if params[:footer]
       update_wiki_page(wiki, page.sidebar, params[:sidebar], commit) if params[:sidebar]
       committer.commit
 
-      page = wiki.page(rename) if rename
-
       redirect to("/#{page.escaped_url_path}") unless page.nil?
     end
 
@@ -298,7 +330,6 @@ module Precious
         @page = page
         @name = name
         @content = page.formatted_data
-        @editable = true
         mustache :page
       else
         halt 404

lib/gollum/frontend/public/gollum/css/dialog.css

@@ -87,6 +87,16 @@
     font-family: 'Monaco', 'Courier New', Courier, monospace;
   }
 
+  #gollum-dialog-dialog-body fieldset .field span.context {
+    font-size: .9em;
+    color: #666;
+  }
+    #gollum-dialog-dialog-body fieldset .field span.context span.path {
+      font-family: 'Monaco', 'Courier New', Courier, monospace;
+      font-weight: bold;
+    }
+
+
 #gollum-dialog-dialog-body fieldset .field:last-child {
   margin: 0 0 1em 0;
 }

lib/gollum/frontend/public/gollum/javascript/gollum.dialog.js

@@ -79,6 +79,10 @@
           fieldAttributes.id + '">';
       }
 
+      if( fieldAttributes.context ){
+        html += '<span class="context">' + fieldAttributes.context + '</span>';
+      }
+
       return html;
     },
 

lib/gollum/frontend/public/gollum/javascript/gollum.js

@@ -1,10 +1,44 @@
+// Helpers
+function pageName(){
+  // "my/dir/file" => "file"
+  return typeof(pageFullPath) == 'undefined' ? undefined : pageFullPath.split('/').pop();
+}
+function pagePath(){
+  // "my/dir/file" => "my/dir"
+  return typeof(pageFullPath) == 'undefined' ? undefined : pageFullPath.split('/').slice(0,-1).join('/');
+}
+
+// Generic HTML escape function
+function htmlEscape( str ) {
+  // The (slower) alternative is: return $('<div/>').text(str).html();
+  // http://stackoverflow.com/questions/1219860/javascript-jquery-html-encoding/7124052#7124052
+  return String(str)
+    .replace(/&/g, '&amp;')
+    .replace(/"/g, '&quot;')
+    .replace(/'/g, '&#39;')
+    .replace(/</g, '&lt;')
+    .replace(/>/g, '&gt;');
+}
+
+// Given a page name and a current path, returns a fully qualified path.
+function abspath(path, name){
+  // Make sure the given path starts at the root.
+  if(name[0] != '/'){
+    name = '/' + path + '/' + name;
+  }
+  var name_parts = name.split('/');
+  var newPath = name_parts.slice(0, -1).join('/');
+  var newName = name_parts.pop();
+  // return array of [path, name]
+  return [newPath, newName];
+}
+
 // ua
 $(document).ready(function() {
   $('#delete-link').click( function(e) {
     var ok = confirm($(this).data('confirm'));
     if ( ok ) {
-      var loc = window.location;
+      var loc = baseUrl + '/delete/' + pageFullPath;
-      loc = baseUrl + '/delete' + loc.pathname.replace(baseUrl,'');
       window.location = loc;
     }
     // Don't navigate on cancel.
@@ -113,11 +147,12 @@ $(document).ready(function() {
     $('#minibutton-rename-page').click(function(e) {
       e.preventDefault();
 
-      // Path name without the leading slash.
+      var path = pagePath();
-      var pathname = window.location.pathname.substr(1);
+      var oldName = pageName();
-      var slashIndex = pathname.lastIndexOf('/');
+      var context_blurb = 
-      var oldName = pathname.substr(slashIndex + 1)
+        "Renamed page will be under " +
-      var path = pathname.substr(0, slashIndex);
+        "<span class='path'>" + htmlEscape('/' + path) + "</span>" +
+        " unless an absolute path is given."
 
       $.GollumDialog.init({
         title: 'Rename Page',
@@ -126,7 +161,8 @@ $(document).ready(function() {
             id:   'name',
             name: 'Rename to',
             type: 'text',
-            defaultValue: oldName || ''
+            defaultValue: oldName || '',
+            context: context_blurb
           }
         ],
         OK: function( res ) {
@@ -134,16 +170,17 @@ $(document).ready(function() {
           if ( res['name'] ) {
             newName = res['name'];
           }
+          var name_parts = abspath(path, newName);
+          var newPath = name_parts[0];
 
-          var msg = 'Renamed ' + oldName + ' to ' + newName;
+          var msg = '/' + path == newPath ? 'Renamed ' + oldName + ' to ' + newName
-          jQuery.ajax( {
+                                          : 'Renamed ' + oldName + ' to ' + name_parts.join('/');
-            type: 'POST',
+          // Fill in the rename form
-            url: baseUrl + '/edit/' + oldName,
+          // This is preferable to AJAX so that we automatically follow the 302 response.
-            data:  { path: path, rename: newName, page: oldName, message: msg },
+          var rename_form = $("form[name=rename]");
-            success: function() {
+          rename_form.children("input[name=rename]").val(name_parts.join('/'));
-                window.location = baseUrl + '/' + encodeURIComponent(newName);
+          rename_form.children("input[name=message]").val(msg);
-            }
+          rename_form.submit();
-          });
         }
       });
     });
@@ -154,6 +191,23 @@ $(document).ready(function() {
     $('#minibutton-new-page').click(function(e) {
       e.preventDefault();
 
+      var path = pagePath();
+      if( path === undefined && $('#file-browser').length != 0 ){
+        // In the pages view, pageFullPath isn't defined.
+        // The new button will still expect a value however.
+        // So we try to figure one out from window.location
+        path = baseUrl == '' ? window.location.pathname.substr(1)
+                             : window.location.pathname.substr(baseUrl.length + 1);
+        // Remove the page viewer part of the url.
+        path = path.replace(/^pages\/?/,'')
+        // For consistency remove the trailing /
+        path = path.replace(/\/$/,'')
+      }
+      var context_blurb = 
+        "Page will be created under " +
+        "<span class='path'>" + htmlEscape('/' + path) + "</span>" +
+        " unless an absolute path is given."
+
       $.GollumDialog.init({
         title: 'Create New Page',
         fields: [
@@ -161,7 +215,8 @@ $(document).ready(function() {
             id:   'name',
             name: 'Page Name',
             type: 'text',
-            defaultValue: ''
+            defaultValue: '',
+            context: context_blurb
           }
         ],
         OK: function( res ) {
@@ -169,7 +224,13 @@ $(document).ready(function() {
           if ( res['name'] ) {
             name = res['name'];
           }
-          window.location = baseUrl + '/' + encodeURIComponent(name);
+          var name_encoded = [];
+          var name_parts = abspath(path, name).join('/').split('/');
+          // Split and encode each component individually.
+          for( var i=0; i < name_parts.length; i++ ){
+            name_encoded.push(encodeURIComponent(name_parts[i]));
+          }
+          window.location = baseUrl + name_encoded.join('/');
         }
       });
     });

lib/gollum/frontend/templates/layout.mustache

@@ -12,7 +12,12 @@
   <link rel="stylesheet" type="text/css" href="{{base_url}}/css/ie7.css" media="all">
   <![endif]-->
 
-  <script>var baseUrl = '{{base_url}}'</script>
+  <script>
+      var baseUrl = '{{base_url}}';
+      {{#page}}
+      var pageFullPath = '{{url_path}}';
+      {{/page}}
+  </script>
   <script type="text/javascript" src="{{base_url}}/javascript/jquery-1.7.2.min.js"></script>
   <script type="text/javascript" src="{{base_url}}/javascript/mousetrap.min.js"></script>
   <script type="text/javascript" src="{{base_url}}/javascript/gollum.js"></script>

lib/gollum/frontend/templates/page.mustache

@@ -20,9 +20,9 @@ Mousetrap.bind(['e'], function( e ) {
     class="action-all-pages">Files</a></li>
     <li class="minibutton" class="jaws">
       <a href="#" id="minibutton-new-page">New</a></li>
+    {{#editable}}
     <li class="minibutton" class="jaws">
       <a href="#" id="minibutton-rename-page">Rename</a></li>
-    {{#editable}}
     <li class="minibutton"><a href="{{base_url}}/edit/{{escaped_url_path}}"
        class="action-edit-page">Edit</a></li>
     {{/editable}}
@@ -73,3 +73,8 @@ Mousetrap.bind(['e'], function( e ) {
   </p>
 </div>
 </div>
+
+<form name="rename" method="POST" action="{{base_url}}/rename/{{escaped_url_path}}">
+  <input type="hidden" name="rename"/>
+  <input type="hidden" name="message"/>
+</from>

lib/gollum/wiki.rb

@@ -312,6 +312,62 @@ module Gollum
       multi_commit ? committer : committer.commit
     end
 
+    # Public: Rename an existing page without altering content.
+    #
+    # page   - The Gollum::Page to update.
+    # rename - The String extension-less full path of the page (leading '/' is ignored).
+    # commit - The commit Hash details:
+    #          :message   - The String commit message.
+    #          :name      - The String author full name.
+    #          :email     - The String email address.
+    #          :parent    - Optional Grit::Commit parent to this update.
+    #          :tree      - Optional String SHA of the tree to create the
+    #                       index from.
+    #          :committer - Optional Gollum::Committer instance.  If provided,
+    #                       assume that this operation is part of batch of
+    #                       updates and the commit happens later.
+    #
+    # Returns the String SHA1 of the newly written version, or the
+    # Gollum::Committer instance if this is part of a batch update.
+    # Returns false if the operation is a NOOP.
+    def rename_page(page, rename, commit = {})
+      return false if page.nil?
+      return false if rename.nil? or rename.empty?
+
+      (target_dir, target_name) = ::File.split(rename)
+      (source_dir, source_name) = ::File.split(page.path)
+      source_name = page.filename_stripped
+
+      # File.split gives us relative paths with ".", commiter.add_to_index doesn't like that.
+      target_dir = '' if target_dir == '.'
+      source_dir = '' if source_dir == '.'
+      target_dir = target_dir.gsub(/^\//, '')
+
+      # if the rename is a NOOP, abort
+      if source_dir == target_dir and source_name == target_name
+        return false
+      end
+
+      multi_commit = false
+      committer = if obj = commit[:committer]
+        multi_commit = true
+        obj
+      else
+        Committer.new(self, commit)
+      end
+
+      committer.delete(page.path)
+      committer.add_to_index(target_dir, target_name, page.format, page.raw_data, :allow_same_ext)
+
+      committer.after_commit do |index, sha|
+        @access.refresh
+        index.update_working_dir(source_dir, source_name, page.format)
+        index.update_working_dir(target_dir, target_name, page.format)
+      end
+
+      multi_commit ? committer : committer.commit
+    end
+
     # Public: Update an existing page with new content. The location of the
     # page inside the repository will not change. If the given format is
     # different than the current format of the page, the filename will be

test/examples/revert.git/config

@@ -4,6 +4,8 @@
 	bare = false
 	logallrefupdates = true
 	ignorecase = true
+[receive]
+    denyCurrentBranch = ignore
 [remote "origin"]
 	fetch = +refs/heads/*:refs/remotes/origin/*
 	url = /Users/rick/p/gollum/test/examples/revert.git

test/examples/revert.git/logs/HEAD

@@ -1,3 +1,4 @@
 0000000000000000000000000000000000000000 7c45b5f16ff3bae2a0063191ef832701214d4df5 rick <technoweenie@gmail.com> 1291942707 -0800	clone: from /Users/rick/p/gollum/test/examples/revert.git
 7c45b5f16ff3bae2a0063191ef832701214d4df5 f403b791119f8232b7cb0ba455c624ac6435f433 rick <technoweenie@gmail.com> 1291942743 -0800	commit: add footer and sidebar
 f403b791119f8232b7cb0ba455c624ac6435f433 ed6c9f63b98acf73c25b5ffbb38da557d3682023 bootstraponline <cafe@bootstraponline.com> 1336421777 -0600	commit: Add header.
+ed6c9f63b98acf73c25b5ffbb38da557d3682023 084a558a1fb3cded23129e2dfad3a17d07d73fd3 Daniel Kimsey <dekimsey@ufl.edu> 1354899095 -0500	push

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

@@ -1,3 +1,4 @@
 0000000000000000000000000000000000000000 7c45b5f16ff3bae2a0063191ef832701214d4df5 rick <technoweenie@gmail.com> 1291942707 -0800	clone: from /Users/rick/p/gollum/test/examples/revert.git
 7c45b5f16ff3bae2a0063191ef832701214d4df5 f403b791119f8232b7cb0ba455c624ac6435f433 rick <technoweenie@gmail.com> 1291942743 -0800	commit: add footer and sidebar
 f403b791119f8232b7cb0ba455c624ac6435f433 ed6c9f63b98acf73c25b5ffbb38da557d3682023 bootstraponline <cafe@bootstraponline.com> 1336421777 -0600	commit: Add header.
+ed6c9f63b98acf73c25b5ffbb38da557d3682023 084a558a1fb3cded23129e2dfad3a17d07d73fd3 Daniel Kimsey <dekimsey@ufl.edu> 1354899095 -0500	push

test/examples/revert.git/objects/08/4a558a1fb3cded23129e2dfad3a17d07d73fd3

@@ -0,0 +1,3 @@
+x
ŤŹMjĂ0…»Ö)ć
Š4cËPJŮőŇü´˘±]dy‘Ű'� ‹÷ń6ĽÇë<×�đĄ7U0‹‰PŚÉgĂ ‰
+Ł Qz„Xl˘˘Ĺýĺ¦K•�'b™Rf#*dVJL’‰F‰C
+>D—÷ţł6¸äĄęľęĽé
ŢDŹö±Űő¤˛żĂ9Ž‘’Gxőä˝ăcaק݄çéßuź"*�aŰ‹Ô¦Ü×v�®[?P—oxĽČłnî'űS

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

@@ -1 +1 @@
-ed6c9f63b98acf73c25b5ffbb38da557d3682023
+084a558a1fb3cded23129e2dfad3a17d07d73fd3

test/test_app.rb

@@ -153,22 +153,78 @@ context "Frontend" do
   end
 
   test "renames page" do
-    page_1 = @wiki.page('B')
+    page_1 = @wiki.page("B")
-    post "/edit/B", :content => 'abc',
+    post "/rename/B", :rename => "/C", :message => 'def'
-      :rename => "C", :page => 'B',
+
-      :format => page_1.format, :message => 'def'
     follow_redirect!
-    assert_equal '/c', last_request.fullpath
+    assert_equal '/C', last_request.fullpath
     assert last_response.ok?
 
     @wiki.clear_cache
     assert_nil @wiki.page("B")
     page_2 = @wiki.page('C')
-    assert_equal 'abc', page_2.raw_data
+    assert_equal "INITIAL\n\nSPAM2\n", page_2.raw_data
     assert_equal 'def', page_2.version.message
     assert_not_equal page_1.version.sha, page_2.version.sha
   end
 
+  test "renames page catches invalid page" do
+    # No such page
+    post "/rename/no-such-file-here", :rename => "/C", :message => 'def'
+    assert !last_response.ok?
+    assert_equal last_response.status, 500
+  end
+
+  test "rename page catches empty target" do
+    # Empty rename target
+    post "/rename/B", :rename => "", :message => 'def'
+    assert !last_response.ok?
+    assert_equal last_response.status, 500
+  end
+
+  test "rename page catches non-existent target" do
+    # Non-existent rename target
+    post "/rename/B", :message => 'def'
+    assert !last_response.ok?
+    assert_equal last_response.status, 500
+  end
+
+
+  test "renames page in subdirectory" do
+    page_1 = @wiki.paged("H", "G")
+    assert_not_equal page_1, nil
+    post "/rename/G/H", :rename => "/I/C", :message => 'def'
+
+    follow_redirect!
+    assert_equal '/I/C', last_request.fullpath
+    assert last_response.ok?
+
+    @wiki.clear_cache
+    assert_nil @wiki.paged("H", "G")
+    page_2 = @wiki.paged('C', 'I')
+    assert_equal "INITIAL\n\nSPAM2\n", page_2.raw_data
+    assert_equal 'def', page_2.version.message
+    assert_not_equal page_1.version.sha, page_2.version.sha
+  end
+
+  test "renames page relative in subdirectory" do
+    page_1 = @wiki.paged("H", "G")
+    assert_not_equal page_1, nil
+    post "/rename/G/H", :rename => "K/C", :message => 'def'
+
+    follow_redirect!
+    assert_equal '/G/K/C', last_request.fullpath
+    assert last_response.ok?
+
+    @wiki.clear_cache
+    assert_nil @wiki.paged("H", "G")
+    page_2 = @wiki.paged('C', 'G/K')
+    assert_equal "INITIAL\n\nSPAM2\n", page_2.raw_data
+    assert_equal 'def', page_2.version.message
+    assert_not_equal page_1.version.sha, page_2.version.sha
+  end
+
+
   test "creates page" do
     post "/create", :content => 'abc', :page => "D",
       :format => 'markdown', :message => 'def'
@@ -212,6 +268,16 @@ context "Frontend" do
     assert last_response.ok?
   end
 
+  test "edit returns nil for non-existant page" do
+    # post '/edit' fails. post '/edit/' works.
+    page = 'not-real-page'
+    path = '/'
+    post '/edit/', :content => 'edit_msg',
+      :page => page, :path => path, :message => ''
+    page_e = @wiki.paged(page, path)
+    assert_equal nil, page_e
+  end
+
   test "page create and edit with dash & page rev" do
     page = 'c-d-e'
     path = 'a/b/' # path must end with /

test/test_wiki.rb

@@ -527,3 +527,136 @@ context "Wiki page writing with different branch" do
     assert_equal nil, @wiki.page("Bilbo")
   end
 end
+
+context "Renames directory traversal" do
+  setup do
+    @path = cloned_testpath("examples/revert.git")
+    @wiki = Gollum::Wiki.new(@path)
+    Precious::App.set(:gollum_path, @path)
+    Precious::App.set(:wiki_options, {})
+  end
+
+  teardown do
+    FileUtils.rm_rf(@path)
+  end
+
+  test "rename aborts on nil" do
+    cd = {:message => "def"}
+    res = @wiki.rename_page(@wiki.page("some-super-fake-page"), "B", cd)
+    assert !res, "rename did not abort with non-existant page"
+    res = @wiki.rename_page(@wiki.page("B"), "", cd)
+    assert !res, "rename did not abort with empty rename"
+    res = @wiki.rename_page(@wiki.page("B"), nil, cd)
+    assert !res, "rename did not abort with nil rename"
+  end
+
+  test "rename page no-act" do
+    # Make sure renames don't do anything if the name is the same.
+    cd = {:message => "def"}
+
+    # B.md => B.md
+    res = @wiki.rename_page(@wiki.page("B"), "B", cd)
+    assert !res, "NOOP rename did not abort"
+  end
+
+  test "rename page without directories" do
+    # Make sure renames work with relative paths.
+    cd = {:message => "def"}
+    source = @wiki.page("B")
+
+    # B.md => C.md
+    res = @wiki.rename_page(source, "C", cd)
+    assert res
+
+    renamed_ok(source, @wiki.page("C"))
+  end
+
+  test "rename page with subdirs" do
+    # Make sure renames in subdirectories happen ok
+    cd = {:message => "def"}
+    source = @wiki.paged("H", "G")
+
+    # G/H.md => G/F.md
+    @wiki.rename_page(source, "G/F", cd)
+
+    renamed_ok(source, @wiki.paged("F", "G"))
+  end
+
+  test "rename page absolute path is still no-act" do
+    # Make sure renames don't do anything if the name is the same.
+    cd = {:message => "def"}
+
+    # B.md => B.md
+    res = @wiki.rename_page(@wiki.page("B"), "/B", cd)
+    assert !res, "NOOP rename did not abort"
+  end
+
+  test "rename page absolute path NOOPs ok" do
+    # Make sure renames don't do anything if the name is the same and we are in a subdirectory.
+    cd = {:message => "def"}
+    source = @wiki.paged("H", "G")
+
+    # G/H.md => G/H.md
+    res = @wiki.rename_page(source, "/G/H", cd)
+    assert !res, "NOOP rename did not abort"
+  end
+
+  test "rename page absolute directory" do
+    # Make sure renames work with absolute paths.
+    cd = {:message => "def"}
+    source = @wiki.page("B")
+
+    # B.md => C.md
+    res = @wiki.rename_page(source, "/C", cd)
+    assert res
+
+    renamed_ok(source, @wiki.page("C"))
+  end
+
+  test "rename page absolute directory with subdirs" do
+    # Make sure renames in subdirectories happen ok
+    cd = {:message => "def"}
+    source = @wiki.paged("H", "G")
+
+    # G/H.md => G/F.md
+    @wiki.rename_page(source, "/G/F", cd)
+
+    renamed_ok(source, @wiki.paged("F", "G"))
+  end
+
+  test "rename page relative directory with new dir creation" do
+    # Make sure renames in subdirectories create more subdirectories ok
+    cd = {:message => "def"}
+    source = @wiki.paged("H", "G")
+
+    # G/H.md => G/K/F.md
+    assert_not_equal k = @wiki.rename_page(source, "K/F", cd), false
+
+    new_page = @wiki.paged("F", "K")
+    assert_not_equal new_page, nil
+    renamed_ok(source, new_page)
+  end
+
+  test "rename page absolute directory with subdir creation" do
+    # Make sure renames in subdirectories create more subdirectories ok
+    cd = {:message => "def"}
+    source = @wiki.paged("H", "G")
+
+    # G/H.md => G/K/F.md
+    assert_not_equal @wiki.rename_page(source, "/G/K/F", cd), false
+
+    new_page = @wiki.paged("F", "G/K")
+    assert_not_equal new_page, nil
+    renamed_ok(source, new_page)
+  end
+
+  def renamed_ok(page_source, page_target)
+    @wiki.clear_cache
+    page1 = @wiki.paged(page_source.name, page_source.path)
+    assert_nil page1
+    assert_equal "INITIAL\n\nSPAM2\n", page_target.raw_data
+    assert_equal 'def', page_target.version.message
+    assert_not_equal page_source.version.sha, page_target.version.sha
+  end
+end
+