diff --git a/src/libkomwm.py b/src/libkomwm.py index 2a85102..ac6992b 100644 --- a/src/libkomwm.py +++ b/src/libkomwm.py @@ -89,6 +89,7 @@ Priorities ranges' rendering order overview: - BG-by-size: landcover areas sorted by their size ''' +# TODO: Implement better error handling validation_errors_count = 0 def to_boolean(s): @@ -104,6 +105,9 @@ def mwm_encode_color(colors, st, prefix='', default='black'): if prefix: prefix += "-" opacity = hex(255 - int(255 * float(st.get(prefix + "opacity", 1)))) + # TODO: Refactoring idea: here color is converted from float to hex. While MapCSS class + # reads colors from *.mapcss files and converts to float. How about changing MapCSS + # to keep hex values and avoid Hex->Float->Hex operations? color = whatever_to_hex(st.get(prefix + 'color', default))[1:] result = int(opacity + color, 16) colors.add(result) @@ -118,6 +122,7 @@ def mwm_encode_image(st, prefix='icon', bgprefix='symbol'): return False # strip last ".svg" handle = st.get(prefix + "image")[:-4] + # TODO: return `handle` only once return handle, handle @@ -454,6 +459,7 @@ def get_drape_priority(cl, dr_type, object_id, auto_dr_type = None, auto_comment return 0 +# TODO: Split large function to smaller ones def komap_mapswithme(options): if options.data and os.path.isdir(options.data): ddir = options.data @@ -464,6 +470,7 @@ def komap_mapswithme(options): class_order = [] class_tree = {} + # TODO: Introduce new function to parse `colors.txt` for better testability colors_file_name = os.path.join(ddir, 'colors.txt') colors = set() if os.path.exists(colors_file_name): @@ -472,6 +479,7 @@ def komap_mapswithme(options): colors.add(int(colorLine)) colors_in_file.close() + # TODO: Introduce new function to parse `patterns.txt` for better testability patterns = [] def addPattern(dashes): if dashes and dashes not in patterns: @@ -488,9 +496,11 @@ def komap_mapswithme(options): types_file = open(os.path.join(ddir, 'types.txt'), "w") # The mapcss-mapping.csv format is described inside the file itself. + # TODO: introduce new function to parse 'mapcss-mapping.csv' for better testability cnt = 1 unique_types_check = set() - for row in csv.reader(open(os.path.join(ddir, 'mapcss-mapping.csv')), delimiter=';'): + mapping_file = open(os.path.join(ddir, 'mapcss-mapping.csv')) + for row in csv.reader(mapping_file, delimiter=';'): if len(row) <= 1 or row[0].startswith('#'): # Allow for empty lines and comment lines starting with '#'. continue @@ -537,6 +547,7 @@ def komap_mapswithme(options): print("mapswithme", file=types_file) class_tree[cl] = row[0] class_order.sort() + mapping_file.close() types_file.close() output = '' @@ -554,8 +565,10 @@ def komap_mapswithme(options): for i, t in enumerate(v.keys()): mapcss_static_tags[t] = mapcss_static_tags.get(t, True) and i == 0 + # TODO: Introduce new function to parse `mapcss-dynamic.txt` for better testability # Get all mapcss dynamic tags from mapcss-dynamic.txt - mapcss_dynamic_tags = set([line.rstrip() for line in open(os.path.join(ddir, 'mapcss-dynamic.txt'))]) + with open(os.path.join(ddir, 'mapcss-dynamic.txt')) as dynamic_file: + mapcss_dynamic_tags = set([line.rstrip() for line in dynamic_file]) # Parse style mapcss global style @@ -579,6 +592,7 @@ def komap_mapswithme(options): style.finalize_choosers_tree() + # TODO: Introduce new function to work with colors for better testability # Get colors section from style style_colors = {} raw_style_colors = style.get_colors() @@ -616,6 +630,7 @@ def komap_mapswithme(options): all_draw_elements = set() + # TODO: refactor next for-loop for readability and testability global validation_errors_count for results in imapfunc(query_style, ((cl, classificator[cl], options.minzoom, options.maxzoom) for cl in class_order)): for result in results: @@ -920,6 +935,7 @@ def komap_mapswithme(options): return -1 viskeys.sort(key=functools.cmp_to_key(cmprepl)) + # TODO: Introduce new function to dump `visibility.txt` and `classificator.txt` for better testability visibility_file = open(os.path.join(ddir, 'visibility.txt'), "w") classificator_file = open(os.path.join(ddir, 'classificator.txt'), "w") @@ -942,11 +958,13 @@ def komap_mapswithme(options): visibility_file.close() classificator_file.close() + # TODO: Introduce new function to dump `colors.txt` for better testability colors_file = open(colors_file_name, "w") for c in sorted(colors): colors_file.write("%d\n" % (c)) colors_file.close() + # TODO: Introduce new function to dump `patterns.txt` for better testability patterns_file = open(patterns_file_name, "w") for p in patterns: patterns_file.write("%s\n" % (' '.join(str(elem) for elem in p))) diff --git a/src/mapcss/Condition.py b/src/mapcss/Condition.py index a3b84cb..50034ed 100644 --- a/src/mapcss/Condition.py +++ b/src/mapcss/Condition.py @@ -24,7 +24,7 @@ class Condition: params = (params,) self.params = params # e.g. ('highway','primary') if typez == "regex": - self.regex = re.compile(self.params[0], re.I) + self.regex = re.compile(self.params[1], re.I) def extract_tag(self): if self.params[0][:2] == "::" or self.type == "regex": diff --git a/src/mapcss/Eval.py b/src/mapcss/Eval.py index 131e6bb..e37e670 100644 --- a/src/mapcss/Eval.py +++ b/src/mapcss/Eval.py @@ -15,6 +15,10 @@ # You should have received a copy of the GNU General Public License # along with kothic. If not, see . +import logging + +logger = logging.getLogger('mapcss.Eval') +logger.setLevel(logging.ERROR) class Eval(): def __init__(self, s='eval()'): @@ -57,6 +61,8 @@ class Eval(): "any": fake_compute, "min": fake_compute, "max": fake_compute, + "cond": fake_compute, + "boolean": fake_compute, }) return tags @@ -99,12 +105,15 @@ class Eval(): return "{:.4g}".format(result) return str(result) - except: + except Exception as e: + logger.warning(f"Error evaluating expression `{self.expr_text}`", e) return "" def __repr__(self): return "eval(%s)" % self.expr_text + def __eq__(self, other): + return type(self) == type(other) and self.expr_text == other.expr_text def m_boolean(expr): expr = str(expr) diff --git a/src/mapcss/StyleChooser.py b/src/mapcss/StyleChooser.py index 9d3518e..0f5eb50 100644 --- a/src/mapcss/StyleChooser.py +++ b/src/mapcss/StyleChooser.py @@ -76,6 +76,8 @@ class StyleChooser: The styles property is an array of all the style objects to be drawn if any of the ruleChains evaluate to true. """ + # TODO: use logging for debug logs + def __repr__(self): return "{(%s) : [%s] }\n" % (self.ruleChains, self.styles) @@ -122,6 +124,7 @@ class StyleChooser: return rule.runtime_conditions + # TODO: Rename to "applyStyles" def updateStyles(self, sl, tags, xscale, zscale, filter_by_runtime_conditions): # Are any of the ruleChains fulfilled? rule_and_object_id = self.testChains(tags) @@ -143,6 +146,7 @@ class StyleChooser: for a, b in r.items(): "calculating eval()'s" if type(b) == self.eval_type: + # TODO: Move next block to a separate function combined_style = {} for t in sl: combined_style.update(t) @@ -235,6 +239,7 @@ class StyleChooser: """ adds to this.styles """ + # TODO: move next for-loop to a new method. Don't call it on every style append for r in self.ruleChains: if not self.selzooms: self.selzooms = [r.minZoom, r.maxZoom] diff --git a/src/mapcss/__init__.py b/src/mapcss/__init__.py index 8af9730..cc1713a 100644 --- a/src/mapcss/__init__.py +++ b/src/mapcss/__init__.py @@ -25,6 +25,7 @@ from .Condition import Condition NEEDED_KEYS = set(["width", "casing-width", "casing-width-add", "fill-color", "fill-image", "icon-image", "text", "extrude", "background-image", "background-color", "pattern-image", "shield-color", "symbol-shape"]) +# TODO: Unused constant WHITESPACE = re.compile(r'\s+ ', re.S | re.X) COMMENT = re.compile(r'\/\* .*? \*\/ \s* ', re.S | re.X) @@ -40,29 +41,30 @@ VARIABLE_SET = re.compile(r'@([a-z][\w\d]*) \s* : \s* (.+?) \s* ; \s* ', re.S | UNKNOWN = re.compile(r'(\S+) \s* ', re.S | re.X) ZOOM_MINMAX = re.compile(r'(\d+)\-(\d+) $', re.S | re.X) -ZOOM_MIN = re.compile(r'(\d+)\- $', re.S | re.X) -ZOOM_MAX = re.compile(r' \-(\d+) $', re.S | re.X) +ZOOM_MIN = re.compile(r'(\d+)\- $', re.S | re.X) +ZOOM_MAX = re.compile(r' \-(\d+) $', re.S | re.X) ZOOM_SINGLE = re.compile(r' (\d+) $', re.S | re.X) -CONDITION_TRUE = re.compile(r'\s* ([:\w]+) \s* [?] \s* $', re.I | re.S | re.X) +# TODO: move to Condition.py +CONDITION_TRUE = re.compile(r'\s* ([:\w]+) \s* [?] \s* $', re.I | re.S | re.X) CONDITION_invTRUE = re.compile(r'\s* [!] \s* ([:\w]+) \s* [?] \s* $', re.I | re.S | re.X) -CONDITION_FALSE = re.compile(r'\s* ([:\w]+) \s* = \s* no \s* $', re.I | re.S | re.X) -CONDITION_SET = re.compile(r'\s* ([-:\w]+) \s* $', re.S | re.X) -CONDITION_UNSET = re.compile(r'\s* !([:\w]+) \s* $', re.S | re.X) -CONDITION_EQ = re.compile(r'\s* ([:\w]+) \s* = \s* (.+) \s* $', re.S | re.X) -CONDITION_NE = re.compile(r'\s* ([:\w]+) \s* != \s* (.+) \s* $', re.S | re.X) -CONDITION_GT = re.compile(r'\s* ([:\w]+) \s* > \s* (.+) \s* $', re.S | re.X) -CONDITION_GE = re.compile(r'\s* ([:\w]+) \s* >= \s* (.+) \s* $', re.S | re.X) -CONDITION_LT = re.compile(r'\s* ([:\w]+) \s* < \s* (.+) \s* $', re.S | re.X) -CONDITION_LE = re.compile(r'\s* ([:\w]+) \s* <= \s* (.+) \s* $', re.S | re.X) -CONDITION_REGEX = re.compile(r'\s* ([:\w]+) \s* =~\/ \s* (.+) \/ \s* $', re.S | re.X) +CONDITION_FALSE = re.compile(r'\s* ([:\w]+) \s* = \s* no \s* $', re.I | re.S | re.X) +CONDITION_SET = re.compile(r'\s* ([-:\w]+) \s* $', re.S | re.X) +CONDITION_UNSET = re.compile(r'\s* !([:\w]+) \s* $', re.S | re.X) +CONDITION_EQ = re.compile(r'\s* ([:\w]+) \s* = \s* (.+) \s* $', re.S | re.X) +CONDITION_NE = re.compile(r'\s* ([:\w]+) \s* != \s* (.+) \s* $', re.S | re.X) +CONDITION_GT = re.compile(r'\s* ([:\w]+) \s* > \s* (.+) \s* $', re.S | re.X) +CONDITION_GE = re.compile(r'\s* ([:\w]+) \s* >= \s* (.+) \s* $', re.S | re.X) +CONDITION_LT = re.compile(r'\s* ([:\w]+) \s* < \s* (.+) \s* $', re.S | re.X) +CONDITION_LE = re.compile(r'\s* ([:\w]+) \s* <= \s* (.+) \s* $', re.S | re.X) +CONDITION_REGEX = re.compile(r'\s* ([:\w]+) \s* =~\/ \s* (.+) \/ \s* $', re.S | re.X) ASSIGNMENT_EVAL = re.compile(r"\s* (\S+) \s* \: \s* eval \s* \( \s* ' (.+?) ' \s* \) \s* $", re.I | re.S | re.X) -ASSIGNMENT = re.compile(r'\s* (\S+) \s* \: \s* (.+?) \s* $', re.S | re.X) -SET_TAG_EVAL = re.compile(r"\s* set \s+(\S+)\s* = \s* eval \s* \( \s* ' (.+?) ' \s* \) \s* $", re.I | re.S | re.X) -SET_TAG = re.compile(r'\s* set \s+(\S+)\s* = \s* (.+?) \s* $', re.I | re.S | re.X) -SET_TAG_TRUE = re.compile(r'\s* set \s+(\S+)\s* $', re.I | re.S | re.X) -EXIT = re.compile(r'\s* exit \s* $', re.I | re.S | re.X) +ASSIGNMENT = re.compile(r'\s* (\S+) \s* \: \s* (.+?) \s* $', re.S | re.X) +SET_TAG_EVAL = re.compile(r"\s* set \s+(\S+)\s* = \s* eval \s* \( \s* ' (.+?) ' \s* \) \s* $", re.I | re.S | re.X) +SET_TAG = re.compile(r'\s* set \s+(\S+)\s* = \s* (.+?) \s* $', re.I | re.S | re.X) +SET_TAG_TRUE = re.compile(r'\s* set \s+(\S+)\s* $', re.I | re.S | re.X) +EXIT = re.compile(r'\s* exit \s* $', re.I | re.S | re.X) oNONE = 0 oZOOM = 2 @@ -73,6 +75,7 @@ oDECLARATION = 6 oSUBPART = 7 oVARIABLE_SET = 8 +# TODO: Following block of variables is never used DASH = re.compile(r'\-/g') COLOR = re.compile(r'color$/') BOLD = re.compile(r'^bold$/i') @@ -81,6 +84,7 @@ UNDERLINE = re.compile(r'^underline$/i') CAPS = re.compile(r'^uppercase$/i') CENTER = re.compile(r'^center$/i') +# TODO: Remove unused HEX variable HEX = re.compile(r'^#([0-9a-f]+)$/i') VARIABLE = re.compile(r'@([a-z][\w\d]*)') @@ -98,6 +102,7 @@ class MapCSS(): self.choosers_by_type = {} self.choosers_by_type_zoom_tag = {} self.variables = {} + self.unused_variables = set() self.style_loaded = False def parseZoom(self, s): @@ -110,6 +115,7 @@ class MapCSS(): elif ZOOM_SINGLE.match(s): return float(ZOOM_SINGLE.match(s).groups()[0]), float(ZOOM_SINGLE.match(s).groups()[0]) else: + # TODO: Should we raise an exception here? logging.error("unparsed zoom: %s" % s) def build_choosers_tree(self, clname, type, cltag): @@ -163,6 +169,8 @@ class MapCSS(): runtime_rules.append(runtime_conditions) return runtime_rules + # TODO: Renamed to `get_styles` because it returns a list of styles for each class `::XXX` + # Refactoring idea: Maybe return dict with `object-id` as a key def get_style(self, clname, type, tags, zoom, xscale, zscale, filter_by_runtime_conditions): style = [] if type in self.choosers_by_type_zoom_tag: @@ -207,6 +215,8 @@ class MapCSS(): def get_variable(self, m): name = m.group()[1:] + if name in self.unused_variables: + self.unused_variables.remove(name) if not name in self.variables: raise Exception("Variable not found: " + str(format(name))) return self.variables[name] if name in self.variables else m.group() @@ -219,7 +229,8 @@ class MapCSS(): if filename: basepath = os.path.dirname(filename) if not css: - css = open(filename).read() + with open(filename) as css_file: + css = css_file.read() if not self.style_loaded: self.choosers = [] @@ -339,7 +350,8 @@ class MapCSS(): import_filename = os.path.join(basepath, IMPORT.match(css).groups()[0]) try: css = IMPORT.sub("", css, 1) - import_text = open(import_filename, "r").read() + with open(import_filename, "r") as import_file: + import_text = import_file.read() stck[-1][1] = css # store remained part stck.append([import_filename, import_text, import_text]) wasBroken = True @@ -352,6 +364,7 @@ class MapCSS(): name = VARIABLE_SET.match(css).groups()[0] log.debug("variable set found: %s" % name) self.variables[name] = VARIABLE_SET.match(css).groups()[1] + self.unused_variables.add( name ) css = VARIABLE_SET.sub("", css, 1) previous = oVARIABLE_SET @@ -359,7 +372,7 @@ class MapCSS(): elif UNKNOWN.match(css): raise Exception("Unknown construction: " + UNKNOWN.match(css).group()) - # Must be unreacheable + # Must be unreachable else: raise Exception("Unexpected construction: " + css) @@ -377,10 +390,13 @@ class MapCSS(): css_orig = stck[-1][2] # original css = stck[-1][1] # remained line = css_orig[:-len(css)].count("\n") + 1 + # TODO: Handle filename is None msg = str(e) + "\nFile: " + filename + "\nLine: " + str(line) + # TODO: Print stack trace of original exception `e` raise Exception(msg) try: + # TODO: Drop support of z-index because `clamp` is always False and z-index properties unused in Organic Maps) if clamp: "clamp z-indexes, so they're tightly following integers" zindex = set() @@ -398,8 +414,10 @@ class MapCSS(): else: stylez['z-index'] = res except TypeError: + # TODO: Better error handling here pass + # Group MapCSS styles by object type: 'area', 'line', 'way', 'node' for chooser in self.choosers: for t in chooser.compatible_types: if t not in self.choosers_by_type: @@ -407,7 +425,11 @@ class MapCSS(): else: self.choosers_by_type[t].append(chooser) + if self.unused_variables: + # TODO: Do not print warning here. Instead let libkomwn.komap_mapswithme(...) analyze unused_variables + print(f"Warning: Unused variables: {', '.join(self.unused_variables)}") +# TODO: move to Condition.py def parseCondition(s): log = logging.getLogger('mapcss.parser.condition') @@ -488,7 +510,7 @@ def parseDeclaration(s): logging.debug("%s == %s" % (tzz[0], tzz[1])) else: logging.debug("unknown %s" % (a)) - return [t] + return [t] # TODO: don't wrap `t` dict into a list. Return `t` instead. if __name__ == "__main__":