Skip to content

Commit

Permalink
fix searching for special escaped character on nt os
Browse files Browse the repository at this point in the history 
-use RootPathFile containers to ease testing of escaped file parts
  • Loading branch information
@elbeardmorez
elbeardmorez committed Feb 5, 2018
1 parent 8c5147b commit 9e956c8
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 58 deletions.
58 changes: 32 additions & 26 deletions quodlibet/quodlibet/formats/_audio.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@
from quodlibet import _, print_d
from quodlibet import util
from quodlibet import config
from quodlibet.util.path import mkdir, mtime, expanduser, \
normalize_path, escape_filename, ismount, get_home_dir
from quodlibet.util.path import mkdir, mtime, expanduser, normalize_path, \
ismount, get_home_dir, RootPathFile
from quodlibet.util.string import encode, decode, isascii

from quodlibet.util import iso639
Expand Down Expand Up @@ -572,27 +572,46 @@ def sanitise(sep, parts):

# generate all potential paths (unresolved/unexpanded)
pathfiles = OrderedDict()
for p in lyric_paths:
for r in lyric_paths:
for f in lyric_filenames:
pathfile = os.path.join(p, os.path.dirname(f),
pathfile = os.path.join(r, os.path.dirname(f),
fsnative(os.path.basename(f)))
rpf = RootPathFile(r, pathfile)
if not pathfile in pathfiles:
pathfiles[pathfile] = pathfile
pathfiles[pathfile] = rpf

#print_d("searching for lyrics in:\n%s" % '\n'.join(pathfiles.keys()))

# expand each raw pathfile in turn and test for existence
match_ = ""
pathfiles_expanded = OrderedDict()
rx_params = re.compile('[^\\\]<[^' + re.escape(os.sep) + ']*[^\\\]>')
for pathfile in pathfiles.keys():
path = expanduser(pathfile)
if rx_params.search(path):
path = ArbitraryExtensionFileFromPattern(pathfile).format(self)
pathfiles_expanded[path] = path # resolved as late as possible
if os.path.exists(path):
match_ = path
expand_patterns = ArbitraryExtensionFileFromPattern
for pf, rpf in pathfiles.items():
root = expanduser(rpf.root)
pathfile = expanduser(pf)
if rx_params.search(pathfile):
root = expand_patterns(root).format(self)
pathfile = expand_patterns(pathfile).format(self)
# resolved as late as possible
pathfiles_expanded[pathfile] = RootPathFile(root, pathfile)
if os.path.exists(pathfile):
match_ = pathfile
break
elif os.name == "nt":
# try a special character encoded version
#
# only pass the proposed path through 'escape_filename' (which
# apparently doesn't respect case) if we don't care about case.
# most 'alien' chars are supported for 'nix fs paths too
#
# FIX: assumes 'nix build used on a case-sensitive fs, nt case
# insensitive. clearly this is not biting anyone though (yet!)
pathfile = os.path.sep.join([root, rpf.end_escaped])
pathfiles_expanded[pathfile] = RootPathFile(root, pathfile)
if os.path.exists(pathfile):
match_ = pathfile
break

if not match_:
# search even harder!
Expand Down Expand Up @@ -627,22 +646,9 @@ def generate_mod_ext_paths(pathfile):
break

if not match_:
# default
match_ = list(pathfiles_expanded.keys())[0]

if os.name == "nt":
# FIX: assumes 'nix build used on a case-sensitive fs, nt case
# insensitive, hence only pass the proposed path through
# 'escape_filename' (which apparently doesn't respect case) if
# we don't care about case
#
# FIX: in any case this call misses the possibility of needing to
# escape characters in the file's parent dir if that was built
# from the artist name etc.
#
# clearly this isn't biting anyone though..
match_ = os.path.join(os.path.dirname(match_),
escape_filename(os.path.basename(match_)))

return match_

@property
Expand Down
92 changes: 60 additions & 32 deletions quodlibet/tests/test_formats__audio.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@
from quodlibet.formats._audio import NUMERIC_ZERO_DEFAULT
from quodlibet.formats import decode_value, MusicFile, FILESYSTEM_TAGS
from quodlibet.util.tags import _TAGS as TAGS
from quodlibet.util.path import normalize_path, mkdir, get_home_dir, unquote
from quodlibet.util.path import normalize_path, mkdir, get_home_dir, unquote, \
escape_filename, RootPathFile

from .helper import temp_filename

Expand Down Expand Up @@ -432,19 +433,19 @@ def assertEqual(a, b):
mkdir(p)
with io.open(fp, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
search = unquote(s.lyric_filename)
os.remove(fp)
os.rmdir(p)
assertEqual(fp_searched, fp)
assertEqual(search, fp)

# test built-in default local path
root = os.path.dirname(filename)
fp = os.path.join(root, artist + " - " + title + ".lyric")
with io.open(fp, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
search = s.lyric_filename
os.remove(fp)
assertEqual(fp_searched, fp)
assertEqual(search, fp)

# custom lyrics file location / naming
config.set("memory", "lyric_filenames",
Expand All @@ -454,16 +455,16 @@ def assertEqual(a, b):

# test custom default (fnf fallback!)
fp = os.path.join(root, artist + ".-." + title)
fp_searched = unquote(s.lyric_filename)
assertEqual(fp_searched, fp)
search = unquote(s.lyric_filename)
assertEqual(search, fp)

# test user defined
fp = os.path.join(root, artist + " - " + title + ".lyric")
with io.open(fp, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
search = s.lyric_filename
os.remove(fp)
assertEqual(fp_searched, fp)
assertEqual(search, fp)

# test order priority
root2 = os.path.join(get_home_dir(), ".lyrics") # built-in default
Expand All @@ -476,49 +477,76 @@ def assertEqual(a, b):
with io.open(fp, "w", encoding='utf-8') as f:
f.write(u"")
mkdir(p2)
fp_searched = unquote(s.lyric_filename)
search = s.lyric_filename
os.remove(fp2)
os.rmdir(p2)
os.remove(fp)
assertEqual(fp_searched, fp)
assertEqual(search, fp)

# test modified extension fallback search
fp = os.path.join(root, artist + " - " + title + ".txt")
with io.open(fp, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
search = s.lyric_filename
os.remove(fp)
assertEqual(fp_searched, fp)
assertEqual(search, fp)

# reset to pickup default (no <attr>) templates
config.remove_option("memory", "lyric_filenames")

# test '<' and/or '>' in name
# (not parsed (transparent to test))
for artist in ['\<artist\>', '\<artist>', '<artist\>']:
artist = s['artist'] = artist + " SpongeBob SquarePants"
fp = os.path.join(root, artist + " - " + title + ".lyric")
with io.open(fp, "w", encoding='utf-8') as f:
path_variants = \
['<oldskool>'] if (os.name == "nt") \
else ['\<artist\>', '\<artist>', '<artist\>']
for path_variant in path_variants:
s['artist'] = path_variant + " SpongeBob SquarePants"
parts = [root, s['artist'] + " - " + s['title'] + ".lyric"]
rpf = RootPathFile(root, os.path.sep.join(parts))
if not rpf.is_valid:
rpf = RootPathFile(rpf.root, rpf.pathfile_escaped)
self.assertTrue(rpf.is_valid,
"even escaped target file is not valid")
with io.open(rpf.pathfile, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
os.remove(fp)
assertEqual(fp_searched, fp)
search = s.lyric_filename
os.remove(rpf.pathfile)
assertEqual(search, rpf.pathfile)

# test '<' and '>' in name across path
# (not parsed (transparent to test))
artist = s['artist'] = "a < b"
title = s['title'] = "b > a"
fp = os.path.join(root, artist, title + ".lyric")
p = os.path.dirname(fp)
mkdir(p)
with io.open(fp, "w", encoding='utf-8') as f:
s['artist'] = "a < b"
s['title'] = "b > a"
parts = [root, s['artist'], s['title'] + ".lyric"]
rpf = RootPathFile(root, os.path.sep.join(parts))
rootp = root
rmdirs = []
# ensure valid dir existence
for p in rpf.end.split(os.path.sep)[:-1]:
rootp = os.path.sep.join([root, p])
if not RootPathFile(root, rootp).is_valid:
rootp = os.path.sep.join([root, escape_filename(p)])
self.assertTrue(RootPathFile(root, rootp).is_valid,
"even escaped target dir part is not valid!")
if not os.path.exists(rootp):
mkdir(rootp)
rmdirs.append(rootp)

if not rpf.is_valid:
rpf = RootPathFile(rpf.root, rpf.pathfile_escaped)

with io.open(rpf.pathfile, "w", encoding='utf-8') as f:
f.write(u"")
fp_searched = unquote(s.lyric_filename)
os.remove(fp)
os.rmdir(p)
assertEqual(fp_searched, fp)

# reset config, other tests expect different defaults!
# search for lyric file
search = s.lyric_filename
# clean up test lyric file / path
os.remove(rpf.pathfile)
for p in rmdirs:
os.rmdir(p)
# test whether the 'found' file is the test lyric file
assertEqual(search, rpf.pathfile)

# reset config - other tests expect different defaults!
config.remove_option("memory", "lyric_rootpaths")
config.remove_option("memory", "lyric_filenames")

Expand Down

0 comments on commit 9e956c8

Please sign in to comment.