Skip to content

Commit

Permalink
Improve FilterErrorBuilder.can_build heuristics
Browse files Browse the repository at this point in the history
  • Loading branch information
pederhan committed Nov 27, 2024
1 parent d0176b2 commit 8105501
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 5 deletions.
12 changes: 9 additions & 3 deletions mreg_cli/errorbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

from functools import cache
import logging
import shlex
from abc import ABC, abstractmethod
Expand Down Expand Up @@ -107,12 +108,16 @@ def can_build(self) -> bool: # noqa: D102
return True


DEFAULT_OFFSET = (-1, -1)


class FilterErrorBuilder(ErrorBuilder):
"""Error builder for commands with a filter."""

SUGGESTION = "Consider enclosing this part in quotes."

@staticmethod
@cache
def find_word_with_char_offset(command: str, char: str) -> tuple[int, int]:
"""Find the start and end index of a word containing a specific character.
Expand All @@ -126,7 +131,7 @@ def find_word_with_char_offset(command: str, char: str) -> tuple[int, int]:
"""
command = command.strip()
if not command:
return -1, -1
return DEFAULT_OFFSET

# Parse the command into parts
cmd = shlex.split(command, posix=False) # keep quotes and backslashes
Expand Down Expand Up @@ -154,13 +159,14 @@ def find_word_with_char_offset(command: str, char: str) -> tuple[int, int]:
)
else:
return start, end
return -1, -1
return DEFAULT_OFFSET

def get_offset(self) -> tuple[int, int]: # noqa: D102 (missing docstring [inherit it from parent])
return self.find_word_with_char_offset(self.command, "|")

def can_build(self) -> bool: # noqa: D102 (missing docstring [inherit it from parent])
return "|" in self.command
offset = self.get_offset()
return offset != DEFAULT_OFFSET


BUILDERS: list[type[ErrorBuilder]] = [
Expand Down
15 changes: 13 additions & 2 deletions tests/test_errorbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
[
(
r"permission label_add 192.168.0.0/24 oracle-group ^(db|cman)ora.*\.example\.com$ oracle",
"failed to compile regex",
"Unable to compile regex",
FilterErrorBuilder,
),
(
r"permission network_add 192.168.0.0/24 pg-group ^db(pg|my).*\.example\.com$",
"Unable to compile regex",
FilterErrorBuilder,
),
(
Expand All @@ -35,7 +40,8 @@ def test_get_builder(command: str, exc_or_str: str, expected: type[ErrorBuilder]
assert builder.get_underline(5, 10) == " ^^^^^"


def test_build_error_message() -> None:
def test_build_error_message_regex() -> None:
"""Test the build_error_message function with command with regex argument."""
# Regex as part of the command
assert build_error_message(
r"permission label_add 192.168.0.0/24 oracle-group ^(db|cman)ora.*\.example\.com$ oracle",
Expand All @@ -47,6 +53,9 @@ def test_build_error_message() -> None:
└ Consider enclosing this part in quotes.\
""")


def test_build_error_message_regex_final_part() -> None:
"""Test the build_error_message function with command with regex argument as final argument."""
# Regex as final part of the command
assert build_error_message(
r"permission network_add 192.168.0.0/24 pg-group ^db(pg|my).*\.example\.com$",
Expand All @@ -58,6 +67,8 @@ def test_build_error_message() -> None:
└ Consider enclosing this part in quotes.\
""")


def test_build_error_message_fallback_default() -> None:
# No suggestion for this error
assert build_error_message(
r"permission label_add other_error", "Other error message"
Expand Down

0 comments on commit 8105501

Please sign in to comment.