diff --git a/CHANGELOG.md b/CHANGELOG.md index b50d4201e..454ddb21e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ _When adding new entries to the changelog, please include issue/PR numbers where ### Major changes Support for spatial filters - the spatial filter can be updated during an `init`, `clone` or `checkout` by supplying the option `--spatial-filter=CRS;GEOMETRY` where CRS is a string such as `EPSG:4326` and GEOMETRY is a polygon or multigon specified using WKT or hex-encoded WKB. When a spatial filter is set, the working copy will only contain features that intersect the spatial filter, and changes that happened outside the working copy are not shown to the user unless specifically required. Starting with Kart 0.11.0, only the features that are inside the specified spatial filter are downloaded during a clone. [Spatial filter docs](docs/SPATIAL_FILTERS.md) | [#456](https://github.com/koordinates/kart/issues/456) ### Other changes +* Expanded `--output-format`/`-o` to accept format specifiers; e.g. `-o json:compact`. `kart log` now accepts text formatstrings, e.g. `-o text:%H` [#544](https://github.com/koordinates/kart/issues/544) +* Deprecated `--json-style` in favour of `-o json:{style}` * Bugfix: fixed errors with Postgres working copies when one or more datasets have no CRS defined. [#529](https://github.com/koordinates/kart/issues/529) * Bugfix: better error message when `kart import` fails due to multiple XML metadata files for a single dataset, which Kart does not support [#547](https://github.com/koordinates/kart/issues/547) * When there are two pieces of XML metadata for a single dataset, but one is simply a GDALMultiDomainMetadata wrapping the other, the wrapped version is ignored. [#547](https://github.com/koordinates/kart/issues/547) diff --git a/kart/cli_util.py b/kart/cli_util.py index 25ddf5132..c8ff538e6 100644 --- a/kart/cli_util.py +++ b/kart/cli_util.py @@ -4,6 +4,7 @@ import jsonschema import os import platform +import warnings from pathlib import Path import click @@ -279,3 +280,85 @@ def actual_callback(ctx, param, value): is_eager=is_eager, **kwargs, ) + + +class OutputFormatType(click.ParamType): + """ + An output format. Loosely divided into two parts: '[:]' + e.g. + json + text + geojson + html + json:compact + text:%H + "text:%H %s" + + For text formatstrings, see the 'PRETTY FORMATS' section of `git help log` + """ + + name = "string" + + JSON_STYLE_CHOICES = ("compact", "extracompact", "pretty") + + def __init__(self, *, output_types, allow_text_formatstring): + self.output_types = tuple(output_types) + self.allow_text_formatstring = allow_text_formatstring + + def convert(self, value, param, ctx): + if isinstance(value, tuple): + return value + if ":" in value: + output_type, fmt = value.split(":", 1) + else: + output_type = value + fmt = None + + if output_type not in self.output_types: + self.fail( + "invalid choice: {}. (choose from {})".format( + output_type, ", ".join(self.output_types) + ), + param, + ctx, + ) + fmt = self.validate_fmt(ctx, param, output_type, fmt) + return output_type, fmt + + def validate_fmt(self, ctx, param, output_type, fmt): + fmt = fmt or None + if output_type in ("json", "json-lines", "geojson"): + fmt = fmt or "pretty" + if fmt not in self.JSON_STYLE_CHOICES: + self.fail( + "invalid choice: {}. (choose from {})".format( + fmt, ", ".join(self.JSON_STYLE_CHOICES) + ), + ctx=ctx, + param=param, + ) + return fmt + elif output_type == "text" and self.allow_text_formatstring: + return fmt + if fmt: + self.fail( + f"Output format '{output_type}' doesn't currently accept any qualifiers (got: ':{fmt}'", + ctx=ctx, + param=param, + ) + + +def parse_output_format(output_format, json_style): + output_type, fmt = output_format + if json_style is not None: + warnings.warn( + f"--json-style is deprecated and will be removed in Kart 0.12. use --output-format={output_type}:{json_style} instead", + RemovalInKart012Warning, + ) + if output_type in ("json", "json-lines", "geojson"): + fmt = json_style + return output_type, fmt + + +class RemovalInKart012Warning(UserWarning): + pass diff --git a/kart/conflicts.py b/kart/conflicts.py index 941c05450..85ec4c3b9 100644 --- a/kart/conflicts.py +++ b/kart/conflicts.py @@ -3,6 +3,7 @@ import click +from .cli_util import OutputFormatType, parse_output_format from .crs_util import CoordinateReferenceString from .exceptions import SUCCESS, SUCCESS_WITH_FLAG from .key_filters import RepoKeyFilter @@ -22,7 +23,7 @@ def list_conflicts( merge_index, merge_context, - output_format="text", + output_type="text", conflict_filter=RepoKeyFilter.MATCH_ALL, summarise=0, flat=False, @@ -46,7 +47,7 @@ def list_conflicts( merge_index - MergeIndex object containing the conflicts found. merge_context - MergeContext object containing RepoStructures. - output_format - one of 'text', 'json', 'geojson' + output_type - one of 'text', 'json', 'geojson' summarise - 1 means summarise (names only), 2 means *really* summarise (counts only). categorise - if True, adds another layer between feature and ID which is the type of conflict, eg "edit/edit" flat - if True, put all features at the top level, with the entire path as the key eg: @@ -55,7 +56,7 @@ def list_conflicts( output_dict = {} conflict_output = _CONFLICT_PLACEHOLDER - if output_format == "geojson": + if output_type == "geojson": flat = True # geojson must be flat or it is not valid geojson summarise = 0 @@ -66,7 +67,7 @@ def list_conflicts( for conflict in conflicts: if not summarise: conflict_output = conflict.output( - output_format, include_label=flat, target_crs=target_crs + output_type, include_label=flat, target_crs=target_crs ) if flat: @@ -80,9 +81,9 @@ def list_conflicts( if summarise: output_dict = summarise_conflicts(output_dict, summarise) - if output_format == "text": + if output_type == "text": return conflicts_json_as_text(output_dict) - elif output_format == "geojson": + elif output_type == "geojson": return conflicts_json_as_geojson(output_dict) else: return output_dict @@ -221,7 +222,10 @@ def conflicts_json_as_geojson(json_obj): @click.option( "--output-format", "-o", - type=click.Choice(["text", "json", "geojson", "quiet"]), + type=OutputFormatType( + output_types=["text", "json", "geojson", "quiet"], + allow_text_formatstring=False, + ), default="text", help="Output format. 'quiet' disables all output and implies --exit-code.", ) @@ -233,8 +237,7 @@ def conflicts_json_as_geojson(json_obj): @click.option( "--json-style", type=click.Choice(["extracompact", "compact", "pretty"]), - default="pretty", - help="How to format the output. Only used with `-o json` and `-o geojson`", + help="[deprecated] How to format the output. Only used with `-o json` and `-o geojson`", ) @click.option( "--exit-code", @@ -279,7 +282,9 @@ def conflicts( repo = ctx.obj.get_repo(allowed_states=KartRepoState.MERGING) merge_index = MergeIndex.read_from_repo(repo) - if output_format == "quiet": + output_type, fmt = parse_output_format(output_format, json_style) + + if output_type == "quiet": ctx.exit(SUCCESS_WITH_FLAG if merge_index.conflicts else SUCCESS) merge_context = MergeContext.read_from_repo(repo) @@ -287,19 +292,19 @@ def conflicts( result = list_conflicts( merge_index, merge_context, - output_format, + output_type, conflict_filter, summarise, flat, crs, ) - if output_format == "text": + if output_type == "text": click.echo(result) - elif output_format == "json": - dump_json_output({"kart.conflicts/v1": result}, sys.stdout, json_style) - elif output_format == "geojson": - dump_json_output(result, sys.stdout, json_style) + elif output_type == "json": + dump_json_output({"kart.conflicts/v1": result}, sys.stdout, fmt) + elif output_type == "geojson": + dump_json_output(result, sys.stdout, fmt) if exit_code: ctx.exit(SUCCESS_WITH_FLAG if merge_context.conflicts else SUCCESS) diff --git a/kart/diff.py b/kart/diff.py index 7b48c0c1c..8921e7f3f 100644 --- a/kart/diff.py +++ b/kart/diff.py @@ -2,6 +2,7 @@ import click +from .cli_util import OutputFormatType, parse_output_format from .crs_util import CoordinateReferenceString from .output_util import dump_json_output @@ -54,8 +55,17 @@ def feature_count_diff( @click.option( "--output-format", "-o", - type=click.Choice( - ["text", "json", "geojson", "quiet", "feature-count", "html", "json-lines"] + type=OutputFormatType( + output_types=[ + "text", + "json", + "geojson", + "quiet", + "feature-count", + "html", + "json-lines", + ], + allow_text_formatstring=False, ), default="text", help=( @@ -82,8 +92,7 @@ def feature_count_diff( @click.option( "--json-style", type=click.Choice(["extracompact", "compact", "pretty"]), - default="pretty", - help="How to format the output. Only used with -o json or -o geojson", + help="[deprecated] How to format the output. Only used with -o json or -o geojson", ) @click.option( "--only-feature-count", @@ -124,25 +133,27 @@ def diff( To list only particular conflicts, supply one or more FILTERS of the form [DATASET[:PRIMARY_KEY]] """ + output_type, fmt = parse_output_format(output_format, json_style) + if only_feature_count: return feature_count_diff( ctx, - output_format, + output_type, commit_spec, output_path, exit_code, - json_style, + fmt, only_feature_count, ) from .base_diff_writer import BaseDiffWriter repo = ctx.obj.get_repo(allowed_states=KartRepoState.ALL_STATES) - diff_writer_class = BaseDiffWriter.get_diff_writer_class(output_format) + diff_writer_class = BaseDiffWriter.get_diff_writer_class(output_type) diff_writer = diff_writer_class( - repo, commit_spec, filters, output_path, json_style=json_style, target_crs=crs + repo, commit_spec, filters, output_path, json_style=fmt, target_crs=crs ) diff_writer.write_diff() - if exit_code or output_format == "quiet": + if exit_code or output_type == "quiet": diff_writer.exit_with_code() diff --git a/kart/log.py b/kart/log.py index d03c76588..3642c5d28 100644 --- a/kart/log.py +++ b/kart/log.py @@ -8,7 +8,12 @@ import click import pygit2 -from .cli_util import tool_environment +from .cli_util import ( + tool_environment, + parse_output_format, + OutputFormatType, + RemovalInKart012Warning, +) from .exec import execvp from .exceptions import NotYetImplemented, SubprocessError from .key_filters import RepoKeyFilter @@ -18,10 +23,6 @@ from . import diff_estimation -class RemovalInKart012Warning(UserWarning): - pass - - class PreserveDoubleDash(click.Command): """ Preserves the double-dash ("--") arg from user input. @@ -263,14 +264,16 @@ def convert_user_patterns_to_raw_paths(paths, repo, commits): @click.option( "--output-format", "-o", - type=click.Choice(["text", "json", "json-lines"]), + type=OutputFormatType( + output_types=["text", "json", "json-lines"], + allow_text_formatstring=True, + ), default="text", ) @click.option( "--json-style", type=click.Choice(["extracompact", "compact", "pretty"]), - default="pretty", - help="How to format the output. Only used with --output-format=json", + help="[deprecated] How to format the output. Only used with --output-format=json", ) @click.option( "--dataset-changes", @@ -364,13 +367,16 @@ def log( options, commits, paths = parse_extra_args(repo, args, **kwargs) paths = convert_user_patterns_to_raw_paths(paths, repo, commits) + output_type, fmt = parse_output_format(output_format, json_style) # TODO: should we check paths exist here? git doesn't! - if output_format == "text": + if output_type == "text": + if fmt: + options.append(f"--format={fmt}") git_args = ["git", "-C", repo.path, "log", *options, *commits, "--", *paths] execvp("git", git_args) - elif output_format in ("json", "json-lines"): + elif output_type in ("json", "json-lines"): try: cmd = [ "git", @@ -409,13 +415,13 @@ def log( ) for (commit_id, refs) in commit_ids_and_refs_log ) - if output_format == "json-lines": + if output_type == "json-lines": for item in commit_log: # hardcoded style here; each item must be on one line. dump_json_output(item, sys.stdout, "compact") else: - dump_json_output(commit_log, sys.stdout, json_style) + dump_json_output(commit_log, sys.stdout, fmt) def _parse_git_log_output(lines): diff --git a/kart/meta.py b/kart/meta.py index bae4febfe..96c617f9a 100644 --- a/kart/meta.py +++ b/kart/meta.py @@ -10,6 +10,8 @@ add_help_subcommand, value_optionally_from_text_file, value_optionally_from_binary_file, + OutputFormatType, + parse_output_format, ) from .checkout import reset_wc_if_needed from .exceptions import InvalidOperation, NotYetImplemented, NotFound, NO_CHANGES @@ -46,14 +48,19 @@ def meta(ctx, **kwargs): @click.option( "--output-format", "-o", - type=click.Choice(["text", "json"]), + type=OutputFormatType( + output_types=[ + "text", + "json", + ], + allow_text_formatstring=False, + ), default="text", ) @click.option( "--json-style", type=click.Choice(["extracompact", "compact", "pretty"]), - default="pretty", - help="How to format the JSON output. Only used with -o json", + help="[deprecated] How to format the JSON output. Only used with -o json", ) @click.option("--ref", default="HEAD") @click.argument("dataset", required=False) @@ -84,14 +91,18 @@ def meta_get(ctx, output_format, json_style, ref, dataset, keys): else: all_items[ds.path] = ds.meta_items() - if output_format == "text": + output_type, fmt = parse_output_format(output_format, json_style) + + if output_type == "text": for ds_path, items in all_items.items(): click.secho(ds_path, bold=True) for key, value in items.items(): click.secho(f" {key}", bold=True) value_indent = " " if key.endswith(".json") or not isinstance(value, str): - value = format_json_for_output(value, fp, json_style=json_style) + value = format_json_for_output( + value, fp, json_style=fmt or "pretty" + ) write_with_indent(fp, value, indent=value_indent) elif key.endswith(".wkt"): value = format_wkt_for_output(value, fp) @@ -99,7 +110,7 @@ def meta_get(ctx, output_format, json_style, ref, dataset, keys): else: fp.write(wrap_text_to_terminal(value, indent=value_indent)) else: - dump_json_output(all_items, fp, json_style=json_style) + dump_json_output(all_items, fp, json_style=fmt) def get_meta_items(ds, keys): diff --git a/kart/show.py b/kart/show.py index af78ce84c..560968975 100644 --- a/kart/show.py +++ b/kart/show.py @@ -1,5 +1,6 @@ import click +from .cli_util import OutputFormatType, parse_output_format from .crs_util import CoordinateReferenceString from .repo import KartRepoState from . import diff_estimation @@ -10,9 +11,22 @@ @click.option( "--output-format", "-o", - # note: geojson isn't here because it doesn't work cleanly with multiple datasets. - type=click.Choice( - ["text", "json", "geojson", "quiet", "feature-count", "html", "json-lines"] + type=OutputFormatType( + output_types=[ + "text", + "json", + "geojson", + "quiet", + "feature-count", + "html", + "json-lines", + ], + # TODO: minor thing, but this should really be True. + # `git show --format=%H` works; no particular reason it shouldn't in Kart. + # (except I didn't get around to implementing it. + # it might be easier to do after moving `log` implementation in-house, since we'll + # presumably have some function that interprets the formatstrings.) + allow_text_formatstring=False, ), default="text", help=( @@ -39,8 +53,7 @@ @click.option( "--json-style", type=click.Choice(["extracompact", "compact", "pretty"]), - default="pretty", - help="How to format the output. Only used with -o json", + help="[deprecated] How to format the output. Only used with -o json", ) @click.option( "--only-feature-count", @@ -76,30 +89,32 @@ def show( commit_spec = f"{refish}^?...{refish}" + output_type, fmt = parse_output_format(output_format, json_style) + if only_feature_count: from .diff import feature_count_diff return feature_count_diff( ctx, - output_format, + output_type, commit_spec, output_path, exit_code, - json_style, + fmt, only_feature_count, ) from .base_diff_writer import BaseDiffWriter repo = ctx.obj.get_repo(allowed_states=KartRepoState.ALL_STATES) - diff_writer_class = BaseDiffWriter.get_diff_writer_class(output_format) + diff_writer_class = BaseDiffWriter.get_diff_writer_class(output_type) diff_writer = diff_writer_class( - repo, commit_spec, filters, output_path, json_style=json_style, target_crs=crs + repo, commit_spec, filters, output_path, json_style=fmt, target_crs=crs ) diff_writer.include_target_commit_as_header() diff_writer.write_diff() - if exit_code or output_format == "quiet": + if exit_code or output_type == "quiet": diff_writer.exit_with_code() diff --git a/tests/test_diff.py b/tests/test_diff.py index 5fa127430..024d477ea 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -1700,7 +1700,7 @@ def test_show_polygons_initial(output_format, data_archive_readonly, cli_runner) def test_show_json_format(data_archive_readonly, cli_runner): with data_archive_readonly("points"): - r = cli_runner.invoke(["show", f"-o", "json", "--json-style=compact", "HEAD"]) + r = cli_runner.invoke(["show", f"-o", "json:compact", "HEAD"]) assert r.exit_code == 0, r.stderr # output is compact, no indentation @@ -1712,7 +1712,7 @@ def test_show_json_coloured(data_archive_readonly, cli_runner, monkeypatch): monkeypatch.setattr(kart.output_util, "can_output_colour", always_output_colour) with data_archive_readonly("points"): - r = cli_runner.invoke(["show", f"-o", "json", "--json-style=pretty", "HEAD"]) + r = cli_runner.invoke(["show", f"-o", "json:pretty", "HEAD"]) assert r.exit_code == 0, r.stderr # No asserts about colour codes - that would be system specific. Just a basic check: assert '"kart.diff/v1+hexwkb"' in r.stdout diff --git a/tests/test_meta.py b/tests/test_meta.py index 1e1542313..3ef884ef5 100644 --- a/tests/test_meta.py +++ b/tests/test_meta.py @@ -209,7 +209,7 @@ def test_commit_files_remove_empty(data_archive, cli_runner): def test_commit_files_amend(data_archive, cli_runner): with data_archive("points"): - r = cli_runner.invoke(["log", "--pretty=%s"]) + r = cli_runner.invoke(["log", "--output-format=text:%s"]) assert r.exit_code == 0, r.stderr assert r.stdout.splitlines() == [ "Improve naming on Coromandel East coast", @@ -228,7 +228,7 @@ def test_commit_files_amend(data_archive, cli_runner): ) assert r.exit_code == 0, r.stderr - r = cli_runner.invoke(["log", "--pretty=%t"]) + r = cli_runner.invoke(["log", "--output-format=text:%t"]) assert r.exit_code == 0, r.stderr actual_tree_contents = r.stdout.splitlines() @@ -243,7 +243,7 @@ def test_commit_files_amend(data_archive, cli_runner): ] ) - r = cli_runner.invoke(["log", "--pretty=%s"]) + r = cli_runner.invoke(["log", "--output-format=text:%s"]) assert r.exit_code == 0, r.stderr assert r.stdout.splitlines() == [ "A more informative commit message", @@ -254,13 +254,13 @@ def test_commit_files_amend(data_archive, cli_runner): ) assert myfile == "myfile" - r = cli_runner.invoke(["log", "--pretty=%t"]) + r = cli_runner.invoke(["log", "--output-format=text:%t"]) assert r.exit_code == 0, r.stderr assert r.stdout.splitlines() == actual_tree_contents # --amend without a message just uses the same message as previous commit r = cli_runner.invoke(["commit-files", "--amend", "x=y"]) - r = cli_runner.invoke(["log", "--pretty=%s"]) + r = cli_runner.invoke(["log", "--output-format=text:%s"]) assert r.exit_code == 0, r.stderr assert r.stdout.splitlines() == [ "A more informative commit message",