-
Notifications
You must be signed in to change notification settings - Fork 124
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: Improve xkeyboard-config script #503
base: master
Are you sure you want to change the base?
tests: Improve xkeyboard-config script #503
Conversation
wismill
commented
Sep 5, 2024
- Refactor to more modern Python
- Allow iterating over models too
- Add filter for models and options
- Add “short output” option
- Refactor to more modern Python - Allow iterating over models too - Add filter for models and options - Add “short output” option
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a few comments, none of which are very important
73e1ce3
to
ab679d3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored further. I would like to speed this up too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't say I read it all, just a couple drive-by comments...
cast, | ||
) | ||
|
||
try: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit: I know @whot suggested to use try
/except
for this, though in my experience the sys.version_info
-style check is more common in Python, because it can be statically analyzed and can be automatically be removed when dropping support for old versions, while try
/except
need manual handling.
In this case, another alternative is to unconditionally do this:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from typing_extensions import Self
|
||
DEFAULT_RULES_XML = "@XKB_CONFIG_ROOT@/rules/evdev.xml" | ||
|
||
# Meson needs to fill this in so we can call the tool in the buildir. | ||
EXTRA_PATH = "@MESON_BUILD_ROOT@" | ||
os.environ["PATH"] = ":".join([EXTRA_PATH, os.getenv("PATH")]) | ||
|
||
os.environ["PATH"] = ":".join(filter(None, (EXTRA_PATH, os.getenv("PATH")))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using Python for some time and didn't know filter(None, ...)
was a thing :)
It might be a bit clearer to do
":".join(path for path in (EXTRA_PATH, os.getenv("PATH")) if path)
but the filter is OK too.
except ImportError: | ||
pass | ||
@property | ||
def __iter__(self) -> Generator[str | None, None, None]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def __iter__(self) -> Generator[str | None, None, None]: | |
def __iter__(self) -> Iterator[str | None]: |
(the fact that it's a generator is an implementation detail)