Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug when renaming a page to a subdirectory containing spaces #20

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions lib/gollum-lib/page.rb
Original file line number Diff line number Diff line change
Expand Up @@ -390,16 +390,15 @@ def find(name, version, dir = nil, exact = false)
# Returns a Gollum::Page or nil if the page could not be found.
def find_page_in_tree(map, name, checked_dir = nil, exact = false)
return nil if !map || name.to_s.empty?
if checked_dir = BlobEntry.normalize_dir(checked_dir)
checked_dir.downcase!
end

checked_dir = BlobEntry.normalize_dir(checked_dir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no need to downcase anymore?

checked_dir = '' if exact && checked_dir.nil?
name = ::File.join(checked_dir, name) if checked_dir

map.each do |entry|
next if entry.name.to_s.empty?
next unless checked_dir.nil? || entry.dir.downcase == checked_dir
next unless page_match(name, entry.name)
path = checked_dir ? ::File.join(entry.dir, entry.name) : entry.name
next unless page_match(name, path)
return entry.page(@wiki, @version)
end

Expand Down Expand Up @@ -435,11 +434,11 @@ def tree_path(treemap, tree)
# Compare the canonicalized versions of the two names.
#
# name - The human or canonical String page name.
# filename - the String filename on disk (including extension).
# path - the String path on disk (including file extension).
#
# Returns a Boolean.
def page_match(name, filename)
if match = self.class.valid_filename?(filename)
def page_match(name, path)
if match = self.class.valid_filename?(path)
@wiki.ws_subs.each do |sub|
return true if Page.cname(name).downcase == Page.cname(match, sub).downcase
end
Expand Down
64 changes: 64 additions & 0 deletions test/test_wiki.rb
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,16 @@
assert_renamed source, @wiki.page("C")
end

test "rename page containing space without directories" do
# Make sure renames involving spaces work with relative paths.
source = @wiki.page("B")

# B.md => C D.md
assert @wiki.rename_page(source, "C D", rename_commit_details)

assert_renamed source, @wiki.page("C D")
end

test "rename page with subdirs" do
# Make sure renames in subdirectories happen ok
source = @wiki.paged("H", "G")
Expand All @@ -595,6 +605,16 @@
assert_renamed source, @wiki.paged("F", "G")
end

test "rename page containing space with subdir" do
# Make sure renames involving spaces in subdirectories happen ok
source = @wiki.paged("H", "G")

# G/H.md => G/F H.md
assert @wiki.rename_page(source, "G/F H", rename_commit_details)

assert_renamed source, @wiki.paged("F H", "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.

Expand Down Expand Up @@ -622,6 +642,16 @@
assert_renamed source, @wiki.page("C")
end

test "rename page with space absolute directory" do
# Make sure renames involving spaces work with absolute paths.
source = @wiki.page("B")

# B.md => C D.md
assert @wiki.rename_page(source, "/C D", rename_commit_details)

assert_renamed source, @wiki.page("C D")
end

test "rename page absolute directory with subdirs" do
# Make sure renames in subdirectories happen ok
source = @wiki.paged("H", "G")
Expand All @@ -632,6 +662,16 @@
assert_renamed source, @wiki.paged("F", "G")
end

test "rename page containing space absolute directory with subdir" do
# Make sure renames involving spaces in subdirectories happen ok
source = @wiki.paged("H", "G")

# G/H.md => G/F H.md
assert @wiki.rename_page(source, "/G/F H", rename_commit_details)

assert_renamed source, @wiki.paged("F H", "G")
end

test "rename page relative directory with new dir creation" do
# Make sure renames in subdirectories create more subdirectories ok
source = @wiki.paged("H", "G")
Expand All @@ -644,6 +684,18 @@
assert_renamed source, new_page
end

test "rename page relative directory with new dir creation containing space" do
# Make sure renames involving spaces in subdirectories create more subdirectories ok
source = @wiki.paged("H", "G")

# G/H.md => G/K L/F.md
assert k = @wiki.rename_page(source, "K L/F", rename_commit_details)

new_page = @wiki.paged("F", "K L")
assert_not_nil new_page
assert_renamed source, new_page
end

test "rename page absolute directory with subdir creation" do
# Make sure renames in subdirectories create more subdirectories ok
source = @wiki.paged("H", "G")
Expand All @@ -656,6 +708,18 @@
assert_renamed source, new_page
end

test "rename page absolute directory with subdir creation containing space" do
# Make sure renames involving spaces in subdirectories create more subdirectories ok
source = @wiki.paged("H", "G")

# G/H.md => G/K L/F.md
assert @wiki.rename_page(source, "/G/K L/F", rename_commit_details)

new_page = @wiki.paged("F", "G/K L")
assert_not_nil new_page
assert_renamed source, new_page
end

def assert_renamed(page_source, page_target)
@wiki.clear_cache
assert_nil @wiki.paged(page_source.name, page_source.path)
Expand Down