From b6efcbfc4e2a19e3de71a598d54d3108f6f8f1c1 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Sat, 16 Mar 2024 11:58:36 -0600 Subject: [PATCH] implement IPC parameter conversion Let's use our type annotations to resolve how to lift strings into whatever their right type is. For now, this just works on "normal" base+hand handled compound types. I have implemented stuff for Union and Literal, since we have those kinds of type annotations. However, I have not implemented e.g. lists as a compound type, since as near as I can tell we do not have any exposed commands that take lists (although it would not be hard, I just didn't want to add a bunch of unused code; see the discussion on the issues cited below for some sample code if you're looking at this in the future). I put normal in quotes, because of e.g. the behavior with bool. It's a bit ugly, but... best to only have to write it once. Maybe we should use eval() for some of these instead? But that's even uglier. Anyway. For supporting "custom" types (i.e. ones defined in libqtile somewhere that are more complex than just a constructor that accepts a string), we could do some kind of poking around in the type for a `from_str`, e.g.: >>> from __future__ import annotations >>> class C: ... def from_str(s: str) -> C: ... pass ... >>> class D: ... def foo(c: C): pass ... >>> typing.get_type_hints(D().foo)["c"] >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str") True again, though, we don't have any of these, so no need to define this convention right now. Finally, one annoying part of all this is that all of the type annotations that anyone uses in the qtile code base need to be imported in interface.py, hence the new noqa import. Otherwise, you end up with, 2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62 Exception in event loop: Traceback (most recent call last): File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback rep = self.handler(req) File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call params = typing.get_type_hints(cmd, globalns=globals()) File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints value = _eval_type(value, globalns, localns) File "/usr/lib/python3.10/typing.py", line 327, in _eval_type return t._evaluate(globalns, localns, recursive_guard) File "/usr/lib/python3.10/typing.py", line 694, in _evaluate eval(self.__forward_code__, globalns, localns), File "", line 1, in NameError: name 'ColorType' is not defined this is annoying, but IMO a small price to pay for only having to write this code once. If we end up with circular dependencies or such, we can split this code out into its own module, but for now it seemed to work fine. Fixes #2433 Fixes #2435 Fixes #4737 Signed-off-by: Tycho Andersen --- libqtile/command/interface.py | 74 ++++++++++++++++++++++++++++++----- test/test_command.py | 17 ++++++++ 2 files changed, 82 insertions(+), 9 deletions(-) diff --git a/libqtile/command/interface.py b/libqtile/command/interface.py index 50b11b8490..f539c69039 100644 --- a/libqtile/command/interface.py +++ b/libqtile/command/interface.py @@ -25,17 +25,20 @@ from __future__ import annotations import traceback +import types +import typing from abc import ABCMeta, abstractmethod -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any, Union, get_args, get_origin + +from typing_extensions import Literal from libqtile import ipc from libqtile.command.base import CommandError, CommandException, CommandObject, SelectError from libqtile.command.graph import CommandGraphCall, CommandGraphNode from libqtile.log_utils import logger +from libqtile.utils import ColorType # noqa: F401 if TYPE_CHECKING: - from typing import Any - from libqtile.command.graph import SelectorType SUCCESS = 0 @@ -308,13 +311,66 @@ def call(self, data: tuple[list[SelectorType], str, tuple, dict]) -> tuple[int, return ERROR, "No such command" logger.debug("Command: %s(%s, %s)", name, args, kwargs) + + def lift_arg(typ, arg): + # for stuff like int | None, allow either + if get_origin(typ) is Union: + for t in get_args(typ): + if t == types.NoneType: + # special case None? I don't know what this looks like + # coming over IPC + if arg == "": + return None + if arg is None: + return None + continue + + try: + return lift_arg(t, arg) + except TypeError: + pass + # uh oh, we couldn't lift it to anything + raise TypeError(f"{arg} is not a {typ}") + + # for literals, check that it is one of the valid strings + if get_origin(typ) is Literal: + if arg not in get_args(typ): + raise TypeError(f"{arg} is not one of {get_origin(typ)}") + return arg + + if typ is bool: + # >>> bool("False") is True + # True + # ... but we want it to be false :) + if arg == "True": + return True + if arg == "False": + return False + raise TypeError(f"{arg} is not a bool") + + if typ is Any: + # can't do any lifting if we don't know the type + return arg + + return typ(arg) + + converted_args = [] + converted_kwargs = dict() + + params = typing.get_type_hints(cmd, globalns=globals()) + + # Check if method is bound + if not hasattr(cmd, "__self__"): + converted_args.append(obj) + + for param, arg in zip(params.keys(), args): + converted_args.append(lift_arg(params[param], arg)) + + for k, v in kwargs.items(): + converted_kwargs[k] = lift_arg(params[k], v) + try: - # Check if method is bound - if hasattr(cmd, "__self__"): - return SUCCESS, cmd(*args, **kwargs) - else: - # If not, pass object as first argument - return SUCCESS, cmd(obj, *args, **kwargs) + return SUCCESS, cmd(*converted_args, **converted_kwargs) except CommandError as err: return ERROR, err.args[0] except Exception: diff --git a/test/test_command.py b/test/test_command.py index 08ffd1deb6..fe42e258db 100644 --- a/test/test_command.py +++ b/test/test_command.py @@ -34,6 +34,7 @@ from libqtile.command.base import CommandObject, expose_command from libqtile.command.interface import CommandError from libqtile.confreader import Config +from libqtile.ipc import IPCError from libqtile.lazy import lazy from test.conftest import dualmonitor @@ -88,6 +89,22 @@ def test_layout_filter(manager): assert manager.c.get_groups()["a"]["focus"] == "two" +@call_config +def test_param_hoisting(manager): + manager.test_window("two") + # 'zomg' is not a valid warp command + with pytest.raises(IPCError): + manager.c.window.focus(warp="zomg") + + manager.c.window.focus(warp="False") + + # 'zomg' is not a valid bar position + with pytest.raises(IPCError): + manager.c.hide_show_bar(position="zomg") + + manager.c.hide_show_bar(position="top") + + class FakeCommandObject(CommandObject): @staticmethod @expose_command()