Skip to content

Commit

Permalink
fix(path) return original casing instead of normalized
Browse files Browse the repository at this point in the history
Use normalizing only for matching. Use original casing when applying.
This problem only existed on Windows. Impacted functions:

- path.common_prefix: no longer forces results to lowercase
- path.relpath: no longer forces results to lowercase
- dir.copyfile: no longer creates files in forced lowercase
- dir.movefile: no longer moves files as forced lowercase
- dir.makepath: no longer creates directories as forced lowercase
  • Loading branch information
Tieske committed May 7, 2019
1 parent 66e8599 commit 8239049
Show file tree
Hide file tree
Showing 6 changed files with 131 additions and 24 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@
- `app.require_here` will now properly handle an absolute base path
- `stringx.split` will no longer append an empty match if the number of requested
elements has already been reached.
- `path.common_prefix` and `path.relpath` return the result in the original casing
(only impacted Windows)
- `dir.copyfile`, `dir.movefile`, and `dir.makepath` create the new file/path with
the requested casing, and no longer force lowercase (only impacted Windows)

## 1.6.0 (2018-11-23)

Expand Down
4 changes: 3 additions & 1 deletion lua/pl/app.lua
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ function app.require_here (base)
p = p..path.sep
end
if base then
base = path.normcase(base)
if path.is_windows then
base = base:gsub('/','\\')
end
if path.isabs(base) then
p = base .. path.sep
else
Expand Down
13 changes: 9 additions & 4 deletions lua/pl/dir.lua
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ local function _listfiles(dir,filemode,match)
return makelist(res)
end

--- return a list of all files in a directory which match the a shell pattern.
--- return a list of all files in a directory which match a shell pattern.
-- @string dir A directory. If not given, all files in current directory are returned.
-- @string mask A shell pattern. If not given, all files are returned.
-- @treturn {string} list of files
Expand Down Expand Up @@ -196,8 +196,10 @@ local function file_op (is_copy,src,dest,flag)
-- fallback if there's no Alien, just use DOS commands *shudder*
-- 'rename' involves a copy and then deleting the source.
if not CopyFile then
src = path.normcase(src)
dest = path.normcase(dest)
if path.is_windows then
src = src:gsub("/","\\")
dest = dest:gsub("/","\\")
end
local res, err = execute_command('copy',two_arguments(src,dest))
if not res then return false,err end
if not is_copy then
Expand Down Expand Up @@ -350,7 +352,10 @@ end
-- @raise failure to create
function dir.makepath (p)
assert_string(1,p)
return _makepath(path.normcase(path.abspath(p)))
if path.is_windows then
p = p:gsub("/", "\\")
end
return _makepath(path.abspath(p))
end


Expand Down
29 changes: 21 additions & 8 deletions lua/pl/path.lua
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ function path.normcase(P)
end

--- normalize a path name.
-- A//B, A/./B and A/foo/../B all become A/B.
-- `A//B`, `A/./B`, and `A/foo/../B` all become `A/B`.
-- @string P a file path
function path.normpath(P)
assert_string(1,P)
Expand Down Expand Up @@ -332,18 +332,25 @@ end
function path.relpath (P,start)
assert_string(1,P)
if start then assert_string(2,start) end
local split,normcase,min,append = utils.split, path.normcase, math.min, table.insert
P = normcase(path.abspath(P,start))
local split,min,append = utils.split, math.min, table.insert
P = path.abspath(P,start)
start = start or currentdir()
start = normcase(start)
local compare
if path.is_windows then
P = P:gsub("/","\\")
start = start:gsub("/","\\")
compare = function(v) return v:lower() end
else
compare = function(v) return v end
end
local startl, Pl = split(start,sep), split(P,sep)
local n = min(#startl,#Pl)
if path.is_windows and n > 0 and at(Pl[1],2) == ':' and Pl[1] ~= startl[1] then
return P
end
local k = n+1 -- default value if this loop doesn't bail out!
for i = 1,n do
if startl[i] ~= Pl[i] then
if compare(startl[i]) ~= compare(Pl[i]) then
k = i
break
end
Expand Down Expand Up @@ -394,12 +401,18 @@ end
function path.common_prefix (path1,path2)
assert_string(1,path1)
assert_string(2,path2)
path1, path2 = path.normcase(path1), path.normcase(path2)
-- get them in order!
if #path1 > #path2 then path2,path1 = path1,path2 end
local compare
if path.is_windows then
path1 = path1:gsub("/", "\\")
path2 = path2:gsub("/", "\\")
compare = function(v) return v:lower() end
else
compare = function(v) return v end
end
for i = 1,#path1 do
local c1 = at(path1,i)
if c1 ~= at(path2,i) then
if compare(at(path1,i)) ~= compare(at(path2,i)) then
local cp = path1:sub(1,i-1)
if at(path1,i-1) ~= sep then
cp = path.dirname(cp)
Expand Down
73 changes: 70 additions & 3 deletions tests/test-dir.lua
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ asserteq(test_samples, {
-- Test move files -----------------------------------------

-- Create a dummy file
local fileName = path.tmpname()
local fileName = path.tmpname() .. "Xx"
file.write( fileName, string.rep( "poot ", 1000 ) )

local newFileName = path.tmpname()
local newFileName = path.tmpname() .. "Xx"
local err, msg = dir.movefile( fileName, newFileName )

-- Make sure the move is successful
Expand All @@ -54,6 +54,18 @@ asserteq( path.exists( fileName ), false )
-- Check to make sure the new file is there
asserteq( path.exists( newFileName ) , newFileName )

-- Test existence again, but explicitly check for correct casing
local files = dir.getfiles(path.dirname(newFileName))
local found = false
for i, filename in ipairs(files) do
if filename == newFileName then
found = true
break
end
end
assert(found, "file was not found in directory, check casing: " .. newFileName)


-- Try to move the original file again (which should fail)
local newFileName2 = path.tmpname()
local err, msg = dir.movefile( fileName, newFileName2 )
Expand All @@ -69,7 +81,7 @@ file.delete( newFileName )
local fileName = path.tmpname()
file.write( fileName, string.rep( "poot ", 1000 ) )

local newFileName = path.tmpname()
local newFileName = path.tmpname() .. "xX"
local err, msg = dir.copyfile( fileName, newFileName )

-- Make sure the move is successful
Expand All @@ -78,6 +90,18 @@ assert( err, msg )
-- Check to make sure the new file is there
asserteq( path.exists( newFileName ) , newFileName )

-- Test existence again, but explicitly check for correct casing
local files = dir.getfiles(path.dirname(newFileName))
local found = false
for i, filename in ipairs(files) do
if filename == newFileName then
found = true
break
end
end
assert(found, "file was not found in directory, check casing: " .. newFileName)


-- Try to move a non-existant file (which should fail)
local fileName2 = 'blub'
local newFileName2 = 'snortsh'
Expand All @@ -89,6 +113,49 @@ file.delete( fileName )
file.delete( newFileName )



-- Test make directory -----------------------------------------

-- Create a dummy file
local dirName = path.tmpname() .. "xX"
local fullPath = dirName .. "/and/one/more"
if path.is_windows then
fullPath = fullPath:gsub("/", "\\")
end
local err, msg = dir.makepath(fullPath)

-- Make sure the move is successful
assert( err, msg )

-- Check to make sure the new file is there
assert(path.isdir(dirName))
assert(path.isdir(fullPath))

-- Test existence again, but explicitly check for correct casing
local files = dir.getdirectories(path.dirname(path.tmpname()))
local found = false
for i, filename in ipairs(files) do
if filename == dirName then
found = true
break
end
end
assert(found, "dir was not found in directory, check casing: " .. newFileName)


-- Try to move a non-existant file (which should fail)
local fileName2 = 'blub'
local newFileName2 = 'snortsh'
local err, msg = dir.copyfile( fileName2, newFileName2 )
asserteq( err, false )

-- Clean up the files
file.delete( fileName )
file.delete( newFileName )




-- have NO idea why forcing the return code is necessary here (Windows 7 64-bit)
os.exit(0)

32 changes: 24 additions & 8 deletions tests/test-path2.lua
Original file line number Diff line number Diff line change
@@ -1,23 +1,39 @@
local path = require 'pl.path'
local test = require 'pl.test'

relpath = path.relpath

path = '/a/b/c'
local asserteq = test.asserteq

function slash (p)
return (p:gsub('\\','/'))
end


-- path.relpath


local testpath = '/a/B/c'

function try (p,r)
test.asserteq(slash(relpath(p,path)),r)
asserteq(slash(path.relpath(p,testpath)),r)
end

try('/a/b/c/one.lua','one.lua')
try('/a/b/c/bonzo/two.lua','bonzo/two.lua')
try('/a/b/three.lua','../three.lua')
try('/a/B/c/one.lua','one.lua')
try('/a/B/c/bonZO/two.lua','bonZO/two.lua')
try('/a/B/three.lua','../three.lua')
try('/a/four.lua','../../four.lua')
try('one.lua','one.lua')
try('../two.lua','../two.lua')


-- path.common_prefix


asserteq(slash(path.common_prefix("../anything","../anything/goes")),"../anything")
asserteq(slash(path.common_prefix("../anything/goes","../anything")),"../anything")
asserteq(slash(path.common_prefix("../anything/goes","../anything/goes")),"../anything")
asserteq(slash(path.common_prefix("../anything/","../anything/")),"../anything")
asserteq(slash(path.common_prefix("../anything","../anything")),"..")
asserteq(slash(path.common_prefix("/hello/world","/hello/world/filename.doc")),"/hello/world")
asserteq(slash(path.common_prefix("/hello/filename.doc","/hello/filename.doc")),"/hello")
if path.is_windows then
asserteq(path.common_prefix("c:\\hey\\there","c:\\hey"),"c:\\hey")
end

0 comments on commit 8239049

Please sign in to comment.