Skip to content

Commit

Permalink
implement IPC parameter conversion
Browse files Browse the repository at this point in the history
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"]
    <class '__main__.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 "<string>", line 1, in <module>
    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 qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <[email protected]>
  • Loading branch information
tych0 committed Mar 30, 2024
1 parent 7bb9ded commit d2ab6e2
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 9 deletions.
90 changes: 81 additions & 9 deletions libqtile/command/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -308,13 +311,82 @@ 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)
try:
# Check if method is bound
if hasattr(cmd, "__self__"):
return SUCCESS, cmd(*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())
logger.error(f"name {name} cmd {cmd} params {params} args {args}")

non_return_annotated_args = filter(lambda k: k != "return", params.keys())
for param, arg in zip(non_return_annotated_args, args):
converted_args.append(lift_arg(params[param], arg))

# if not all args were annotated, we need to keep them anyway. note
# that mixing some annotated and not annotated args will not work well:
# we will reorder args here and cause problems. this is solveable but
# somewhat ugly, and we can avoid it by always annotating all
# parameters.
logger.error(f"{converted_args} {args}")
if len(converted_args) < len(args):
converted_args.extend(args[len(converted_args):])
logger.error(f"{converted_args} {args}")

# Check if method is bound
if not hasattr(cmd, "__self__"):
converted_args.insert(0, obj)

for k, v in kwargs.items():
# if this kwarg has a type annotation, use it
if k in params:
converted_kwargs[k] = lift_arg(params[k], v)
else:
# If not, pass object as first argument
return SUCCESS, cmd(obj, *args, **kwargs)
converted_kwargs[k] = v

try:
return SUCCESS, cmd(*converted_args, **converted_kwargs)
except CommandError as err:
return ERROR, err.args[0]
except Exception:
Expand Down
17 changes: 17 additions & 0 deletions test/test_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit d2ab6e2

Please sign in to comment.