Refactor wlr-resize-watcher to use systemd user service - #3
Refactor wlr-resize-watcher to use systemd user service#3assisted-by-ai wants to merge 1 commit into
Conversation
|
wlr-resize-watcher already is a systemd user unit, and has been for a long time (since 2026-01-28). This is therefore probably not necessary. |
Inherit the full process environment for wlr-randr (via os.environ.copy() with LC_ALL=C override) instead of a restrictive allowlist that omitted WAYLAND_DISPLAY, preventing wlr-randr from connecting to the compositor. Handle disconnected displays in wlr-randr output gracefully instead of aborting with an error when no active mode is found for them. Fix current_mode_re regex to also match a space before the closing delimiter (e.g. "current )" in addition to "current,)" and "current)"). Use flexible mode line parsing — match "current" anywhere in trailing fields rather than requiring exactly 5 space-separated columns. Remove unnecessary f-string wrappers around "\n".join() in error output. https://claude.ai/code/session_01UqYWCHRZZw16ZEa4CAHuSp
d1331d2 to
19d7fa9
Compare
ArrayBolt3
left a comment
ArrayBolt3
left a comment
There was a problem hiding this comment.
Accepted a bit of this, but mostly rejected.
| whitespace_start_re: Pattern[str] = re.compile(r"^\s+") | ||
| modes_re: Pattern[str] = re.compile(r"\s+Modes:$") | ||
| current_mode_re: Pattern[str] = re.compile(r".*[( ]current[,)].*") | ||
| current_mode_re: Pattern[str] = re.compile(r".*[( ]current[ ,)].*") |
There was a problem hiding this comment.
The extra space is unnecessary, this regex matches an item in a comma+space separated list of words in parentheses, meaning that the character before the word may be an opening parenthesis or a space, and the character after the word may be a comma or a closing parenthesis, as is currently written.
| wlr_randr_env: dict[str, str] = os.environ.copy() | ||
| wlr_randr_env["LC_ALL"] = "C" | ||
| wlr_randr_lines: list[str] = subprocess.run( | ||
| ["/usr/bin/wlr-randr"], | ||
| env={ | ||
| "XDG_RUNTIME_DIR": f"{os.environ["XDG_RUNTIME_DIR"]}", | ||
| "LC_ALL": "C", | ||
| }, | ||
| env=wlr_randr_env, |
| file=sys.stderr, | ||
| ) | ||
| print(f"{"\n".join(wlr_randr_lines)}", file=sys.stderr) | ||
| print("\n".join(wlr_randr_lines), file=sys.stderr) |
| print(f"{"\n".join(wlr_randr_lines)}", file=sys.stderr) | ||
| print("\n".join(wlr_randr_lines), file=sys.stderr) |
| out_list: list[DisplayInfo] = [] | ||
| disp_name: str | None = None | ||
| disp_mode: str | None = None | ||
| disp_disconnected: bool = False |
There was a problem hiding this comment.
Rejected, along with all the rest of the disconnected stuff. The word "disconnected" doesn't occur even once in the entire wlr-randr codebase, so trying to detect a disconnected display by looking for it almost certainly won't work. There is such a thing as a display not being enabled, and that does seem to not be handled properly yet, but that requires different code.
There was a problem hiding this comment.
Implemented handling of the display enabled key from wlr-randr here: ArrayBolt3@892049a
| if line.strip() == "": | ||
| continue |
There was a problem hiding this comment.
Rejected, this is unnecessary loosening of input validation, which may be dangerous.
| if len(line_parts) < 4: | ||
| if len(line_parts) < 2: | ||
| print( | ||
| "ERROR: Too few fields in wlr-randr mode " | ||
| "specification! wlr-randr output:", | ||
| file=sys.stderr, | ||
| ) | ||
| print(f"{"\n".join(wlr_randr_lines)}", file=sys.stderr) | ||
| print("\n".join(wlr_randr_lines), file=sys.stderr) | ||
| sys.exit(1) | ||
| if len(line_parts) == 4: | ||
| ## This mode specification is not the active one for the | ||
| ## current display, skip it | ||
| trailing_fields = " ".join(line_parts[1:]) | ||
| if not GlobalData.current_mode_re.match(trailing_fields): | ||
| continue | ||
| if GlobalData.current_mode_re.match(line_parts[4]): | ||
| disp_mode = line_parts[0] | ||
| disp_mode = line_parts[0] |
There was a problem hiding this comment.
Loosening the validation here is undesirable, it could cause dangerous misbehavior. Better to crash on weird output so that users report a bug and we can adapt for that particular output. Rejected.
| if disp_name is None: | ||
| return out_list | ||
|
|
||
| if disp_disconnected: | ||
| return out_list | ||
|
|
||
| if disp_mode is None: | ||
| print( | ||
| "ERROR: Unable to find active display mode for a screen in " | ||
| "wlr-randr output! wlr-randr output:", | ||
| file=sys.stderr, | ||
| ) | ||
| print("\n".join(wlr_randr_lines), file=sys.stderr) | ||
| sys.exit(1) | ||
|
|
||
| out_list.append(DisplayInfo(disp_name, disp_mode)) |
There was a problem hiding this comment.
This is part of the already-rejected "disconnected" handling code.
3 participants
Summary
This PR refactors the wlr-resize-watcher to run as a systemd user service instead of relying solely on XDG autostart, and improves environment variable handling for wlr-randr subprocess calls.
Key Changes
Added systemd user service: Created
wlr-resize-watcher.servicethat binds to the graphical session target, providing better lifecycle management and integration with the Wayland session.Centralized environment variable management: Introduced
GlobalData.wlr_randr_envdictionary to store and reuse environment variables (XDG_RUNTIME_DIR,WAYLAND_DISPLAY,LC_ALL) across all subprocess calls to wlr-randr, replacing inline environment dictionaries.Updated autostart desktop entry: Modified the XDG autostart file to trigger the systemd service via
systemctl --user restartinstead of directly executing the binary.Initialized environment variables in main(): Added initialization of
GlobalData.wlr_randr_envin themain()function to ensure all required Wayland environment variables are captured at startup.Implementation Details
WAYLAND_DISPLAYin addition toXDG_RUNTIME_DIRandLC_ALL, ensuring proper Wayland session context for all wlr-randr invocations.BindsTo=graphical-session.targetto ensure the service lifecycle is tied to the Wayland session.https://claude.ai/code/session_01UqYWCHRZZw16ZEa4CAHuSp