Skip to content

Commit

Permalink
Merge pull request #2422 from fonttools/ufolib-speed-up-filename-clas…
Browse files Browse the repository at this point in the history
…h-checking

Use a set for file names for clash checking
  • Loading branch information
madig committed Oct 7, 2021
2 parents df2916a + 0466408 commit a5173b2
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 12 deletions.
5 changes: 2 additions & 3 deletions Lib/fontTools/ufoLib/filenames.py
Expand Up @@ -15,10 +15,9 @@ class NameTranslationError(Exception):
pass


def userNameToFileName(userName: str, existing=[], prefix="", suffix=""):
def userNameToFileName(userName: str, existing=(), prefix="", suffix=""):
"""
existing should be a case-insensitive list
of all existing file names.
`existing` should be a set-like object.
>>> userNameToFileName("a") == "a"
True
Expand Down
23 changes: 14 additions & 9 deletions Lib/fontTools/ufoLib/glifLib.py
Expand Up @@ -10,6 +10,8 @@
glyph data. See the class doc string for details.
"""

from __future__ import annotations

import logging
import enum
from warnings import warn
Expand Down Expand Up @@ -205,7 +207,7 @@ def __init__(
self.glyphNameToFileName = glyphNameToFileNameFunc
self._validateRead = validateRead
self._validateWrite = validateWrite
self._existingFileNames = None
self._existingFileNames: set[str] | None = None
self._reverseContents = None

self.rebuildContents()
Expand Down Expand Up @@ -455,12 +457,12 @@ def writeGlyph(self, glyphName, glyphObject=None, drawPointsFunc=None, formatVer
fileName = self.contents.get(glyphName)
if fileName is None:
if self._existingFileNames is None:
self._existingFileNames = {}
for fileName in self.contents.values():
self._existingFileNames[fileName] = fileName.lower()
fileName = self.glyphNameToFileName(glyphName, self._existingFileNames.values())
self._existingFileNames = {
fileName.lower() for fileName in self.contents.values()
}
fileName = self.glyphNameToFileName(glyphName, self._existingFileNames)
self.contents[glyphName] = fileName
self._existingFileNames[fileName] = fileName.lower()
self._existingFileNames.add(fileName.lower())
if self._reverseContents is not None:
self._reverseContents[fileName.lower()] = glyphName
data = _writeGlyphToBytes(
Expand All @@ -485,9 +487,9 @@ def deleteGlyph(self, glyphName):
fileName = self.contents[glyphName]
self.fs.remove(fileName)
if self._existingFileNames is not None:
del self._existingFileNames[fileName]
self._existingFileNames.remove(fileName.lower())
if self._reverseContents is not None:
del self._reverseContents[self.contents[glyphName].lower()]
del self._reverseContents[fileName.lower()]
del self.contents[glyphName]

# dict-like support
Expand Down Expand Up @@ -573,9 +575,12 @@ def __exit__(self, exc_type, exc_value, exc_tb):
def glyphNameToFileName(glyphName, existingFileNames):
"""
Wrapper around the userNameToFileName function in filenames.py
Note that existingFileNames should be a set for large glyphsets
or performance will suffer.
"""
if existingFileNames is None:
existingFileNames = []
existingFileNames = set()
return userNameToFileName(glyphName, existing=existingFileNames, suffix=".glif")

# -----------------------
Expand Down
2 changes: 2 additions & 0 deletions Tests/ufoLib/glifLib_test.py
Expand Up @@ -159,6 +159,8 @@ def test_conflicting_case_insensitive_file_names(self, tmp_path):
dst.writeGlyph("a", glyph)
dst.writeGlyph("A", glyph)
dst.writeGlyph("a_", glyph)
dst.deleteGlyph("a_")
dst.writeGlyph("a_", glyph)
dst.writeGlyph("A_", glyph)
dst.writeGlyph("i_j", glyph)

Expand Down

0 comments on commit a5173b2

Please sign in to comment.