From 456a6e1f5a85d000d457efd425c1b1c131c69d3e Mon Sep 17 00:00:00 2001 From: sgraaf Date: Mon, 15 Jun 2026 20:46:17 +0200 Subject: [PATCH] Add `--strict` option to CLI commands --- CHANGELOG.md | 1 + README.md | 6 ++ docs/cli.md | 25 ++++++--- src/gpx/cli.py | 55 +++++++++++++++++- tests/test_cli.py | 138 ++++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 215 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4bf93bb..31ab55b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,7 @@ The **third number** is for emergencies when we need to start branches for older - New `gpx validate` CLI options: - `--strict`: Treat warnings as failures (non-zero exit code). - `--json`: Output a machine-readable validation report. +- New `--strict` option on the `gpx info`, `gpx edit`, `gpx merge` and `gpx convert` CLI commands. When given, each input is validated against the GPX 1.1 schema first: warnings are printed to stderr and the command aborts (non-zero exit code) if any schema errors are found. For `gpx convert`, validation only applies to GPX input. The default keeps the existing lenient behavior. ### Changed diff --git a/README.md b/README.md index c7d0801..bd58b08 100644 --- a/README.md +++ b/README.md @@ -466,6 +466,12 @@ gpx merge file1.gpx file2.gpx file3.gpx -o merged.gpx gpx convert input.gpx -o output.geojson gpx convert input.gpx -o output.kml gpx convert input.geojson -o output.gpx + +# Validate the input first with --strict (info, edit, merge and convert): +# prints warnings and aborts on schema errors before doing any work +gpx info --strict path/to/file.gpx +gpx edit --strict input.gpx -o output.gpx --simplify 10 +gpx merge --strict file1.gpx file2.gpx -o merged.gpx ``` diff --git a/docs/cli.md b/docs/cli.md index 15e600a..7d68c4c 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -82,7 +82,7 @@ options: ## gpx info -Display information and statistics about a GPX file. +Display information and statistics about a GPX file. Use `--strict` to validate the input against the GPX 1.1 schema first, printing warnings and aborting on schema errors. ## gpx edit -Edit a GPX file with various transformations. +Edit a GPX file with various transformations. Use `--strict` to validate the input against the GPX 1.1 schema first, printing warnings and aborting on schema errors. ## gpx convert -Convert between GPX, GeoJSON, and KML formats. +Convert between GPX, GeoJSON, and KML formats. Use `--strict` to validate GPX input against the GPX 1.1 schema first, printing warnings and aborting on schema errors (ignored for non-GPX input). diff --git a/src/gpx/cli.py b/src/gpx/cli.py index 685320e..312413f 100644 --- a/src/gpx/cli.py +++ b/src/gpx/cli.py @@ -15,7 +15,7 @@ from pathlib import Path from typing import TYPE_CHECKING -from .io import convert_file, read_gpx +from .io import convert_file, detect_format, read_gpx from .operations import ( crop, reduce_precision, @@ -29,7 +29,7 @@ trim, ) from .operations import merge as merge_op -from .validation import ValidationResult +from .validation import InvalidGPXError, ValidationResult from .validation import validate as validate_gpx if TYPE_CHECKING: @@ -215,6 +215,38 @@ def _validate_text_output( return 1 if failed else 0 +def _add_strict_argument(parser: argparse.ArgumentParser) -> None: + """Add the shared ``--strict`` option to a file-reading subcommand.""" + parser.add_argument( + "--strict", + action="store_true", + help="Validate the input against the GPX 1.1 schema first; print " + "warnings and abort on schema errors", + ) + + +def _validate_or_raise(file_path: Path) -> None: + """Validate a GPX file, printing warnings and raising on schema errors. + + Any validation warnings are printed to stderr; any schema errors raise + :class:`~gpx.validation.InvalidGPXError`, which the CLI reports as an error + and exits non-zero. This backs the ``--strict`` option of the file-reading + subcommands, mirroring ``read_gpx(..., strict=True)`` while also surfacing + warnings. + """ + result = validate_gpx(file_path) + + if result.warnings: + n_warnings = len(result.warnings) + noun = "warning" if n_warnings == 1 else "warnings" + print(f"⚠ {file_path}: {n_warnings} {noun}", file=sys.stderr) + for issue in result.warnings: + print(f" {issue}", file=sys.stderr) + + if not result.is_valid: + raise InvalidGPXError(result) + + def _add_info_parser( subparsers: argparse._SubParsersAction[argparse.ArgumentParser], ) -> None: @@ -235,6 +267,7 @@ def _add_info_parser( action="store_true", help="Output information in JSON format", ) + _add_strict_argument(info_parser) info_parser.set_defaults(func=info) @@ -246,6 +279,9 @@ def info(args: argparse.Namespace) -> int: print(f"Error: File not found: {file_path}", file=sys.stderr) return 1 + if args.strict: + _validate_or_raise(file_path) + gpx = read_gpx(file_path) info = _gather_gpx_info(gpx) @@ -696,6 +732,7 @@ def _add_edit_parser( help="Number of decimal places for elevation (e.g., 1)", ) + _add_strict_argument(edit_parser) edit_parser.set_defaults(func=edit) @@ -708,6 +745,9 @@ def edit(args: argparse.Namespace) -> int: # noqa: C901, PLR0912 print(f"Error: File not found: {input_path}", file=sys.stderr) return 1 + if args.strict: + _validate_or_raise(input_path) + gpx = read_gpx(input_path) # Apply crop @@ -852,6 +892,7 @@ def _add_merge_parser( metavar="", dest="output_file", ) + _add_strict_argument(parser) parser.set_defaults(func=merge) @@ -866,6 +907,11 @@ def merge(args: argparse.Namespace) -> int: print(f"Error: File not found: {file_path}", file=sys.stderr) return 1 + # In strict mode, validate every input before merging any of them. + if args.strict: + for file_path in files: + _validate_or_raise(file_path) + # Read and merge all GPX files merged_gpx = merge_op(read_gpx(file_path) for file_path in files) @@ -916,6 +962,7 @@ def _add_convert_parser( choices=["gpx", "geojson", "kml"], help="Output format (default: auto-detect from file extension)", ) + _add_strict_argument(parser) parser.set_defaults(func=convert) @@ -928,6 +975,10 @@ def convert(args: argparse.Namespace) -> int: print(f"Error: File not found: {input_path}", file=sys.stderr) return 1 + # Schema validation only applies to GPX input. + if args.strict and (args.from_format or detect_format(input_path)) == "gpx": + _validate_or_raise(input_path) + try: input_format, output_format = convert_file( input_path, diff --git a/tests/test_cli.py b/tests/test_cli.py index 674ce47..105f7a1 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -895,3 +895,141 @@ def test_convert_gpx_to_gpx_preserves_extensions( # Track point extensions preserved assert gpx.trk[0].trkseg[0].trkpt[0].extensions is not None + + +#: A GPX document with a schema error (an unknown element ````). +_SCHEMA_ERROR_GPX = ( + '\n' + '\n' + ' oops\n' + "\n" +) + +#: A GPX document that is schema-valid but yields a warning (version != 1.1). +_SCHEMA_WARNING_GPX = ( + '\n' + '\n' + ' \n' + "\n" +) + + +class TestStrictMode: + """Tests for the shared ``--strict`` option on file-reading commands.""" + + def test_info_strict_fails_on_error( + self, capsys: pytest.CaptureFixture[str], tmp_path: Path + ) -> None: + """``info --strict`` aborts on a schema error.""" + input_file = tmp_path / "bad.gpx" + input_file.write_text(_SCHEMA_ERROR_GPX) + + result = cli(["info", "--strict", str(input_file)]) + assert result == 1 + captured = capsys.readouterr() + assert "unknown element" in captured.err + + def test_info_lenient_ignores_error(self, tmp_path: Path) -> None: + """Without ``--strict``, schema errors are ignored (lenient default).""" + input_file = tmp_path / "bad.gpx" + input_file.write_text(_SCHEMA_ERROR_GPX) + + result = cli(["info", str(input_file)]) + assert result == 0 + + def test_info_strict_warns_but_succeeds( + self, capsys: pytest.CaptureFixture[str], tmp_path: Path + ) -> None: + """``info --strict`` prints warnings to stderr but does not abort.""" + input_file = tmp_path / "warn.gpx" + input_file.write_text(_SCHEMA_WARNING_GPX) + + result = cli(["info", "--strict", str(input_file)]) + assert result == 0 + captured = capsys.readouterr() + assert "WARNING" in captured.err + assert "GPX File:" in captured.out + + def test_edit_strict_fails_on_error( + self, capsys: pytest.CaptureFixture[str], tmp_path: Path + ) -> None: + """``edit --strict`` aborts on a schema error and writes no output.""" + input_file = tmp_path / "bad.gpx" + input_file.write_text(_SCHEMA_ERROR_GPX) + output_file = tmp_path / "out.gpx" + + result = cli(["edit", "--strict", str(input_file), "-o", str(output_file)]) + assert result == 1 + assert not output_file.exists() + captured = capsys.readouterr() + assert "unknown element" in captured.err + + def test_edit_strict_warns_but_writes(self, tmp_path: Path) -> None: + """``edit --strict`` still writes output when there are only warnings.""" + input_file = tmp_path / "warn.gpx" + input_file.write_text(_SCHEMA_WARNING_GPX) + output_file = tmp_path / "out.gpx" + + result = cli(["edit", "--strict", str(input_file), "-o", str(output_file)]) + assert result == 0 + assert output_file.exists() + + def test_merge_strict_fails_on_error( + self, sample_gpx: GPX, capsys: pytest.CaptureFixture[str], tmp_path: Path + ) -> None: + """``merge --strict`` aborts when any input has a schema error.""" + good_file = tmp_path / "good.gpx" + sample_gpx.write_gpx(good_file) + bad_file = tmp_path / "bad.gpx" + bad_file.write_text(_SCHEMA_ERROR_GPX) + output_file = tmp_path / "out.gpx" + + result = cli( + ["merge", "--strict", str(good_file), str(bad_file), "-o", str(output_file)] + ) + assert result == 1 + assert not output_file.exists() + captured = capsys.readouterr() + assert "unknown element" in captured.err + + def test_merge_strict_valid_inputs(self, sample_gpx: GPX, tmp_path: Path) -> None: + """``merge --strict`` succeeds when all inputs are valid.""" + file1 = tmp_path / "file1.gpx" + file2 = tmp_path / "file2.gpx" + sample_gpx.write_gpx(file1) + sample_gpx.write_gpx(file2) + output_file = tmp_path / "out.gpx" + + result = cli( + ["merge", "--strict", str(file1), str(file2), "-o", str(output_file)] + ) + assert result == 0 + assert output_file.exists() + + def test_convert_strict_fails_on_error( + self, capsys: pytest.CaptureFixture[str], tmp_path: Path + ) -> None: + """``convert --strict`` aborts on a schema error in GPX input.""" + input_file = tmp_path / "bad.gpx" + input_file.write_text(_SCHEMA_ERROR_GPX) + output_file = tmp_path / "out.geojson" + + result = cli(["convert", "--strict", str(input_file), "-o", str(output_file)]) + assert result == 1 + assert not output_file.exists() + captured = capsys.readouterr() + assert "unknown element" in captured.err + + def test_convert_strict_skips_non_gpx_input( + self, sample_gpx: GPX, tmp_path: Path + ) -> None: + """``convert --strict`` ignores validation for non-GPX input.""" + input_file = tmp_path / "input.geojson" + output_file = tmp_path / "out.gpx" + sample_gpx.write_geojson(input_file) + + result = cli(["convert", "--strict", str(input_file), "-o", str(output_file)]) + assert result == 0 + assert output_file.exists()