Add --min-saving to structpad suggest

README.md

@@ -99,6 +99,22 @@ struct Frame: size 56 -> 48 (saves 8)
   bool ok;
 ```
 
+`--min-saving N` is available only with `suggest`; `--min-saving=N` is also supported and the default is 0. N is a whole number written with 1 to 6 decimal digits. A struct that would save fewer than N bytes is left out, so a packed struct and a struct that is already compact appear only when N is 0. When every struct is left out, the output is the one line `no struct saves at least N bytes` (`byte` when N is 1).
+
+```sh
+structpad suggest --min-saving 8 net.sp
+```
+
+```text
+struct Frame: size 56 -> 48 (saves 8)
+  struct Packet head;
+  u8 version;
+  struct Wire tail[2];
+  bool ok;
+```
+
+An invalid value such as `-1` produces `invalid --min-saving "-1"`, and `--min-saving` with `layout` or `check` produces `layout does not take --min-saving` or `check does not take --min-saving`. Both exit with status 2.
+
 Check structs against a padding limit; the default limit is zero bytes:
 
 ```sh

src/structpad/cli.py

@@ -11,10 +11,11 @@
 
 _USAGE = (
     "usage: structpad layout [--abi lp64|ilp32] [--struct NAME]... [--format text|json] FILE\n"
-    "       structpad suggest [--abi lp64|ilp32] FILE\n"
+    "       structpad suggest [--abi lp64|ilp32] [--min-saving N] FILE\n"
     "       structpad check [--abi lp64|ilp32] [--max-padding N] [--ignore NAME]... FILE\n"
 )
 _MAX_PADDING_RE = re.compile(r"[0-9]{1,6}\Z")
+_MIN_SAVING_RE = re.compile(r"[0-9]{1,6}\Z")
 _NAME_RE = re.compile(r"[A-Za-z_][A-Za-z0-9_]{0,63}\Z")
 
 
@@ -29,10 +30,11 @@ def _write_error(stderr: BinaryIO, message: str) -> None:
 
 def _parse_options(
     command: str, arguments: list[str]
-) -> tuple[str, int, str, tuple[str, ...], tuple[str, ...], str] | str:
+) -> tuple[str, int, str, tuple[str, ...], tuple[str, ...], str, int] | str:
     abi = "lp64"
     output_format = "text"
     max_padding = 0
+    min_saving = 0
     ignored: list[str] = []
     selected: list[str] = []
     file_name: str | None = None
@@ -90,6 +92,22 @@ def _parse_options(
             max_padding = int(value)
             continue
 
+        if argument == "--min-saving" or argument.startswith("--min-saving="):
+            if command != "suggest":
+                return f"{command} does not take --min-saving"
+            if argument == "--min-saving":
+                if index + 1 >= len(arguments):
+                    return "--min-saving needs a value"
+                value = arguments[index + 1]
+                index += 2
+            else:
+                value = argument[len("--min-saving=") :]
+                index += 1
+            if _MIN_SAVING_RE.fullmatch(value) is None:
+                return f'invalid --min-saving "{_escape_controls(value)}"'
+            min_saving = int(value)
+            continue
+
         if argument == "--ignore" or argument.startswith("--ignore="):
             if command != "check":
                 return f"{command} does not take --ignore"
@@ -131,7 +149,7 @@ def _parse_options(
 
     if file_name is None:
         return "missing FILE"
-    return abi, max_padding, file_name, tuple(ignored), tuple(selected), output_format
+    return abi, max_padding, file_name, tuple(ignored), tuple(selected), output_format, min_saving
 
 
 def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) -> int:
@@ -152,7 +170,9 @@ def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) ->
     if isinstance(parsed_options, str):
         _write_error(stderr, parsed_options)
         return 2
-    abi, max_padding, file_name, ignored_names, selected_names, output_format = parsed_options
+    abi, max_padding, file_name, ignored_names, selected_names, output_format, min_saving = (
+        parsed_options
+    )
     display = "<stdin>" if file_name == "-" else file_name.replace("\\", "/")
 
     try:
@@ -195,7 +215,7 @@ def run(args: list[str], stdin: BinaryIO, stdout: BinaryIO, stderr: BinaryIO) ->
         stdout.write(renderer(structs, abi, selected_names or None))
         return 0
     if command == "suggest":
-        stdout.write(render_suggest(structs, abi))
+        stdout.write(render_suggest(structs, abi, min_saving))
         return 0
 
     output = render_check(structs, abi, max_padding, ignored)

src/structpad/render.py

@@ -106,30 +106,40 @@ def render_layout_json(
     return (json.dumps(layouts, indent=2) + "\n").encode("utf-8")
 
 
-def render_suggest(structs: Sequence[Struct], abi: str) -> bytes:
-    """Render stable field-order suggestions as UTF-8 bytes with LF endings."""
+def render_suggest(structs: Sequence[Struct], abi: str, min_saving: int = 0) -> bytes:
+    """Render stable field-order suggestions as UTF-8 bytes with LF endings.
+
+    A struct that would save fewer than ``min_saving`` bytes is left out.
+    """
     definitions = _definitions(structs)
     blocks: list[str] = []
     for struct in structs:
         current = calculate_layout(struct, abi, definitions)
         if struct.packed:
-            blocks.append(
-                f"struct {struct.name}: packed, size {current.size}, nothing to reorder"
-            )
+            if min_saving == 0:
+                blocks.append(
+                    f"struct {struct.name}: packed, size {current.size}, nothing to reorder"
+                )
             continue
 
         proposed = suggest(struct, abi, definitions)
         if proposed.fields == struct.fields:
-            blocks.append(f"struct {struct.name}: size {current.size}, already compact")
+            if min_saving == 0:
+                blocks.append(f"struct {struct.name}: size {current.size}, already compact")
             continue
 
         improved = calculate_layout(proposed, abi, definitions)
         saving = current.size - improved.size
+        if saving < min_saving:
+            continue
         lines = [
             f"struct {struct.name}: size {current.size} -> {improved.size} (saves {saving})"
         ]
         lines.extend(f"  {_field_declaration(field)}" for field in proposed.fields)
         blocks.append("\n".join(lines))
+    if not blocks:
+        unit = "byte" if min_saving == 1 else "bytes"
+        return f"no struct saves at least {min_saving} {unit}\n".encode()
     return ("\n\n".join(blocks) + "\n").encode("utf-8")
 
 

tests/test_cli.py

@@ -17,7 +17,7 @@
 
 _USAGE = (
     b"usage: structpad layout [--abi lp64|ilp32] [--struct NAME]... [--format text|json] FILE\n"
-    b"       structpad suggest [--abi lp64|ilp32] FILE\n"
+    b"       structpad suggest [--abi lp64|ilp32] [--min-saving N] FILE\n"
     b"       structpad check [--abi lp64|ilp32] [--max-padding N] [--ignore NAME]... FILE\n"
 )
 _MIXED = (

tests/test_min_saving.py

@@ -0,0 +1,160 @@
+"""Example-based tests for the suggest command's minimum-saving option."""
+
+import importlib
+import sys
+from io import BytesIO
+from pathlib import Path
+
+import pytest
+
+_SOURCE_DIRECTORY = str(Path(__file__).resolve().parents[1] / "src")
+if _SOURCE_DIRECTORY not in sys.path:
+    sys.path.insert(0, _SOURCE_DIRECTORY)
+run = importlib.import_module("structpad.cli").run
+
+_NET_SOURCE = (
+    b"struct Packet {\n"
+    b"    u8 tag;\n"
+    b"    u32 length;\n"
+    b"    u16 flags;\n"
+    b"    char name[10];\n"
+    b"    ptr next;\n"
+    b"}\n"
+    b"\n"
+    b"packed struct Wire {\n"
+    b"    u8 kind;\n"
+    b"    u32 value;\n"
+    b"}\n"
+    b"\n"
+    b"struct Frame {\n"
+    b"    u8 version;\n"
+    b"    struct Packet head;\n"
+    b"    struct Wire tail[2];\n"
+    b"    bool ok;\n"
+    b"}\n"
+)
+_TWO_SAVINGS = (
+    b"struct Small {\n"
+    b"    u8 a;\n"
+    b"    u16 b;\n"
+    b"    u8 c;\n"
+    b"    u16 d;\n"
+    b"}\n"
+    b"\n"
+    b"struct Big {\n"
+    b"    u8 a;\n"
+    b"    u64 b;\n"
+    b"    u8 c;\n"
+    b"    u64 d;\n"
+    b"}\n"
+)
+_FRAME_BLOCK = (
+    b"struct Frame: size 56 -> 48 (saves 8)\n"
+    b"  struct Packet head;\n"
+    b"  u8 version;\n"
+    b"  struct Wire tail[2];\n"
+    b"  bool ok;\n"
+)
+
+
+def _run(args: list[str], source: bytes = _NET_SOURCE) -> tuple[int, bytes, bytes]:
+    stdout = BytesIO()
+    stderr = BytesIO()
+    status = run(args, BytesIO(source), stdout, stderr)
+    return status, stdout.getvalue(), stderr.getvalue()
+
+
+def test_default_output_is_unchanged() -> None:
+    status, default_out, stderr = _run(["suggest", "-"])
+    assert status == 0
+    assert stderr == b""
+    assert default_out == (
+        b"struct Packet: size 32, already compact\n"
+        b"\n"
+        b"struct Wire: packed, size 5, nothing to reorder\n"
+        b"\n" + _FRAME_BLOCK
+    )
+    assert _run(["suggest", "--min-saving", "0", "-"]) == (0, default_out, b"")
+    assert _run(["suggest", "--min-saving=0", "-"]) == (0, default_out, b"")
+
+
+def test_one_byte_leaves_out_compact_and_packed_structs() -> None:
+    assert _run(["suggest", "--min-saving", "1", "-"]) == (0, _FRAME_BLOCK, b"")
+
+
+def test_the_limit_is_inclusive() -> None:
+    assert _run(["suggest", "--min-saving", "8", "-"]) == (0, _FRAME_BLOCK, b"")
+    assert _run(["suggest", "--min-saving=9", "-"]) == (0, b"no struct saves at least 9 bytes\n", b"")
+
+
+def test_one_byte_message_uses_the_singular() -> None:
+    source = b"struct Tight {\n    u32 a;\n}\n"
+    assert _run(["suggest", "--min-saving", "1", "-"], source) == (
+        0,
+        b"no struct saves at least 1 byte\n",
+        b"",
+    )
+
+
+def test_only_structs_with_a_large_enough_saving_remain() -> None:
+    status, small_and_big, _ = _run(["suggest", "--min-saving", "1", "-"], _TWO_SAVINGS)
+    assert status == 0
+    assert small_and_big.startswith(b"struct Small: size 8 -> 6 (saves 2)\n")
+    assert b"struct Big: size 32 -> 24 (saves 8)\n" in small_and_big
+    status, only_big, _ = _run(["suggest", "--min-saving", "3", "-"], _TWO_SAVINGS)
+    assert status == 0
+    assert only_big.startswith(b"struct Big: size 32 -> 24 (saves 8)\n")
+    assert b"Small" not in only_big
+
+
+def test_option_works_with_abi_and_after_the_file() -> None:
+    status, out, stderr = _run(["suggest", "-", "--abi=ilp32", "--min-saving=1"])
+    assert (status, stderr) == (0, b"")
+    assert out == (
+        b"struct Frame: size 40 -> 36 (saves 4)\n"
+        b"  struct Packet head;\n"
+        b"  u8 version;\n"
+        b"  struct Wire tail[2];\n"
+        b"  bool ok;\n"
+    )
+
+
+def test_the_last_value_wins() -> None:
+    assert _run(["suggest", "--min-saving", "9", "--min-saving", "8", "-"]) == (
+        0,
+        _FRAME_BLOCK,
+        b"",
+    )
+
+
+@pytest.mark.parametrize("value", ["1", "999999", "000001", "007"])
+def test_accepts_one_to_six_digits(value: str) -> None:
+    status, _out, stderr = _run(["suggest", f"--min-saving={value}", "-"])
+    assert (status, stderr) == (0, b"")
+
+
+@pytest.mark.parametrize("value", ["", "-1", "+1", "1.0", "x", "1000000", " 1", "1 "])
+def test_rejects_other_values(value: str) -> None:
+    assert _run(["suggest", f"--min-saving={value}", "-"]) == (
+        2,
+        b"",
+        f'structpad: invalid --min-saving "{value}"\n'.encode(),
+    )
+
+
+def test_needs_a_value() -> None:
+    assert _run(["suggest", "--min-saving"]) == (2, b"", b"structpad: --min-saving needs a value\n")
+
+
+@pytest.mark.parametrize("command", ["layout", "check"])
+def test_other_commands_do_not_take_it(command: str) -> None:
+    for args in ([command, "--min-saving", "1", "-"], [command, "--min-saving=1", "-"]):
+        assert _run(args) == (2, b"", f"structpad: {command} does not take --min-saving\n".encode())
+
+
+def test_value_is_escaped_in_the_error() -> None:
+    assert _run(["suggest", "--min-saving=a\nb", "-"]) == (
+        2,
+        b"",
+        b'structpad: invalid --min-saving "a\\nb"\n',
+    )

tests/test_readme.py

@@ -56,6 +56,20 @@ def test_readme_documents_ignore_option() -> None:
         assert required_text in readme
 
 
+def test_readme_documents_min_saving_option() -> None:
+    readme = _readme()
+
+    for required_text in (
+        "`--min-saving N` is available only with `suggest`",
+        "structpad suggest --min-saving 8 net.sp",
+        "no struct saves at least N bytes",
+        'invalid --min-saving "-1"',
+        "layout does not take --min-saving",
+        "check does not take --min-saving",
+    ):
+        assert required_text in readme
+
+
 def test_readme_has_worked_input_and_exact_layout_output_as_fenced_blocks() -> None:
     blocks = _fenced_blocks(_readme())