diff --git a/src/lisflood/settings_tool.py b/src/lisflood/settings_tool.py index 0c55e366..bb7093c8 100644 --- a/src/lisflood/settings_tool.py +++ b/src/lisflood/settings_tool.py @@ -5,11 +5,17 @@ from __future__ import absolute_import import argparse +import re import sys import yaml from lxml import etree +# Built-in path variables that LISFLOOD provides automatically. +_BUILTIN_VARS = frozenset({ + "SettingsPath", "SettingsDir", "ProjectPath", "ProjectDir", +}) + class SettingsToolError(Exception): """Raised when the settings tool cannot complete requested operations.""" @@ -75,6 +81,14 @@ def _flatten_pairs(raw_values): return flattened +_VAR_RE = re.compile(r"\$\(([^)]+)\)") + + +def _find_refs(value): + """Return set of variable names referenced via $(Name) in *value*.""" + return set(_VAR_RE.findall(value)) + + def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None, lfuser_values=None, check=False): parser = etree.XMLParser(remove_blank_text=True, remove_comments=False) tree = etree.parse(input_path, parser) @@ -87,7 +101,7 @@ def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None _ = _get_required_child(root, ("lfbinding", "lfbindings")) if check: - return 0 + return _validate_refs(lfuser_elem, root) if not output_path: raise SettingsToolError("Output path is required for the 'set' subcommand.") @@ -129,6 +143,141 @@ def run_tool(input_path, output_path=None, file_path=None, lfoptions_values=None return 0 +def _validate_refs(lfuser_elem, root): + """Check that every $(Var) reference resolves to a defined lfuser name or a builtin.""" + user_index = _index_textvars(lfuser_elem) + defined = set(user_index.keys()) | _BUILTIN_VARS + errors = [] + + lfbinding_elem = root.find("lfbinding") + if lfbinding_elem is None: + lfbinding_elem = root.find("lfbindings") + + for section_tag, elem in [("lfuser", lfuser_elem), ("lfbinding", lfbinding_elem)]: + if elem is None: + continue + for node in elem.findall(".//textvar"): + name = node.get("name", "?") + value = node.get("value", "") + for ref in _find_refs(value): + if ref not in defined: + errors.append( + "{}: '{}' references undefined variable '$({})'.".format( + section_tag, name, ref + ) + ) + + # Also scan lfoptions — they shouldn't contain $(refs) but flag if they do + lfoptions_elem = root.find("lfoptions") + if lfoptions_elem is not None: + for node in lfoptions_elem.findall(".//setoption"): + choice = node.get("choice", "") + for ref in _find_refs(choice): + errors.append( + "lfoptions: '{}' choice references variable '$({})'.".format( + node.get("name", "?"), ref + ) + ) + + if errors: + for err in errors: + print(" " + err, file=sys.stderr) + raise SettingsToolError("{} undefined variable reference(s) found.".format(len(errors))) + return 0 + + +def _parse_sections(path): + """Parse an XML file and return (tree, root, lfoptions_elem, lfuser_elem, lfbinding_elem).""" + parser = etree.XMLParser(remove_blank_text=True, remove_comments=False) + tree = etree.parse(path, parser) + root = tree.getroot() + if root.tag != "lfsettings": + raise SettingsToolError("Root element must be , found <{}>.".format(root.tag)) + lfoptions_elem = _get_required_child(root, ("lfoptions",)) + lfuser_elem = _get_required_child(root, ("lfuser",)) + lfbinding_elem = _get_required_child(root, ("lfbinding", "lfbindings")) + return tree, root, lfoptions_elem, lfuser_elem, lfbinding_elem + + +def _shell_quote(value): + """Wrap *value* in single quotes so it is safe to paste into a shell command.""" + return "'" + value.replace("'", "'\"'\"'") + "'" + + +def _copy_path(path): + """Return *path* with '_copy' inserted before the extension.""" + import os + base, ext = os.path.splitext(path) + return base + "_copy" + ext + + +def run_diff(input_path, output_path): + """Compare two LISFLOOD settings XMLs and print differences.""" + _, _, opts_a, user_a, _ = _parse_sections(input_path) + _, _, opts_b, user_b, _ = _parse_sections(output_path) + + idx_opts_a = {n.get("name"): n.get("choice") for n in opts_a.findall(".//setoption") if n.get("name")} + idx_opts_b = {n.get("name"): n.get("choice") for n in opts_b.findall(".//setoption") if n.get("name")} + idx_user_a = {n.get("name"): n.get("value") for n in user_a.findall(".//textvar") if n.get("name")} + idx_user_b = {n.get("name"): n.get("value") for n in user_b.findall(".//textvar") if n.get("name")} + + opt_diffs = [] + user_diffs = [] + + all_opt_keys = sorted(set(idx_opts_a) | set(idx_opts_b)) + for key in all_opt_keys: + va = idx_opts_a.get(key) + vb = idx_opts_b.get(key) + if va != vb: + opt_diffs.append((key, va, vb)) + + all_user_keys = sorted(set(idx_user_a) | set(idx_user_b)) + for key in all_user_keys: + va = idx_user_a.get(key) + vb = idx_user_b.get(key) + if va != vb: + user_diffs.append((key, va, vb)) + + if not opt_diffs and not user_diffs: + print("No differences found.") + return 0 + + if opt_diffs: + print("lfoptions:") + for name, va, vb in opt_diffs: + la = " (absent)" if va is None else va + lb = " (absent)" if vb is None else vb + print(" {}: {} -> {}".format(name, la, lb)) + + if user_diffs: + print("lfuser:") + for name, va, vb in user_diffs: + la = " (absent)" if va is None else va + lb = " (absent)" if vb is None else vb + print(" {}: {} -> {}".format(name, la, lb)) + + # Build the set command to go from input (-i) to output (-o) + out_copy = _copy_path(output_path) + cmd_parts = ["lisflood-settings set -i {} -o {}".format( + _shell_quote(input_path), _shell_quote(out_copy) + )] + set_opt_parts = [] + set_user_parts = [] + for name, _va, vb in opt_diffs: + if vb is not None: + set_opt_parts.append(_shell_quote("{}={}".format(name, vb))) + for name, _va, vb in user_diffs: + if vb is not None: + set_user_parts.append(_shell_quote("{}={}".format(name, vb))) + if set_opt_parts: + cmd_parts.append("--lfoptions " + " ".join(set_opt_parts)) + if set_user_parts: + cmd_parts.append("--lfuser " + " ".join(set_user_parts)) + print("\nEquivalent command (-i -> -o):") + print(" " + " ".join(cmd_parts)) + return 0 + + def build_parser(): parser = argparse.ArgumentParser( description="Parse, validate, lint and update LISFLOOD settings XML files." @@ -137,10 +286,17 @@ def build_parser(): check_parser = subparsers.add_parser( "check", - help="Validate XML structure/sections without writing output.", + help="Validate XML structure, sections, and variable references.", ) check_parser.add_argument("-i", "--input", required=True, help="Input LISFLOOD settings XML file.") + diff_parser = subparsers.add_parser( + "diff", + help="Show differences between two settings XMLs.", + ) + diff_parser.add_argument("-i", "--input", required=True, help="First (base) LISFLOOD settings XML file.") + diff_parser.add_argument("-o", "--output", required=True, help="Second (target) LISFLOOD settings XML file.") + set_parser = subparsers.add_parser( "set", help="Apply updates and write output XML.", @@ -182,6 +338,10 @@ def main(argv=None): run_tool(input_path=args.input, check=True) return 0 + if args.command == "diff": + run_diff(args.input, args.output) + return 0 + lfoptions_values = _flatten_pairs(args.lfoptions) lfuser_values = _flatten_pairs(args.lfuser) run_tool( diff --git a/tests/test_settings_tool.py b/tests/test_settings_tool.py index 472cb86e..8d743f49 100644 --- a/tests/test_settings_tool.py +++ b/tests/test_settings_tool.py @@ -1,7 +1,7 @@ import pytest from lxml import etree -from lisflood.settings_tool import main +from lisflood.settings_tool import main, SettingsToolError SAMPLE_XML = """\ @@ -30,6 +30,65 @@ """ +SAMPLE_BAD_REF_XML = """\ + + + + + + + + + + + + + + + + +""" + +SAMPLE_BAD_BINDING_REF_XML = """\ + + + + + + + + + + + + + + + +""" + +SAMPLE_XML_B = """\ + + + + + + + + + + + + + + + + + + + +""" + def _write(path, content): path.write_text(content, encoding="utf-8") @@ -164,3 +223,75 @@ def test_lfoptions_rejects_non_key_value_token(self, tmp_path): with pytest.raises(SystemExit): main(["set", "-i", str(src), "-o", str(dst), "--lfoptions", "wateruse=1", "not-a-pair"]) + + def test_check_validates_refs_ok(self, tmp_path): + src = tmp_path / "settings.xml" + _write(src, SAMPLE_XML) + + rc = main(["check", "-i", str(src)]) + assert rc == 0 + + def test_check_catches_undefined_lfuser_ref(self, tmp_path): + src = tmp_path / "bad.xml" + _write(src, SAMPLE_BAD_REF_XML) + + rc = main(["check", "-i", str(src)]) + assert rc == 1 + + def test_check_catches_undefined_lfbinding_ref(self, tmp_path): + src = tmp_path / "bad.xml" + _write(src, SAMPLE_BAD_BINDING_REF_XML) + + rc = main(["check", "-i", str(src)]) + assert rc == 1 + + def test_check_allows_builtin_vars(self, tmp_path): + xml = """\ + + + + + + + + + + + + + +""" + src = tmp_path / "builtin.xml" + _write(src, xml) + + rc = main(["check", "-i", str(src)]) + assert rc == 0 + + def test_diff_identical_files(self, tmp_path, capsys): + src = tmp_path / "a.xml" + _write(src, SAMPLE_XML) + + rc = main(["diff", "-i", str(src), "-o", str(src)]) + assert rc == 0 + assert "No differences" in capsys.readouterr().out + + def test_diff_shows_changes(self, tmp_path, capsys): + a = tmp_path / "a.xml" + b = tmp_path / "b.xml" + _write(a, SAMPLE_XML) + _write(b, SAMPLE_XML_B) + + rc = main(["diff", "-i", str(a), "-o", str(b)]) + assert rc == 0 + + out = capsys.readouterr().out + assert "TemperatureInKelvin" in out + assert "wateruse" in out + assert "PathRoot" in out + assert "SomeUserVar" in out + assert "Equivalent command" in out + # Generated command uses real paths with _copy suffix + assert str(a) in out + assert "b_copy.xml" in out + # Values are single-quoted for shell safety + assert "'PathRoot=/data/root'" in out