Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion aw_watcher_window/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
from .main import main
def main():
from .main import main as _main

return _main()
Comment on lines +1 to +4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Lazy import may surprise entry-point callers

The wrapper function is needed to avoid pulling in aw_client/aw_core when the package is imported (e.g. during test collection), but it changes the identity of the exported symbol: aw_watcher_window.main.__module__ is now aw_watcher_window instead of aw_watcher_window.main, and attributes like __doc__ from the real main are no longer visible. A comment explaining why the deferred import exists would help future readers understand the intent.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!


__all__ = ["main"]
56 changes: 53 additions & 3 deletions aw_watcher_window/macos.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ var baseurl = "http://localhost:5600"
var clientHostname = ProcessInfo.processInfo.hostName
var clientName = "aw-watcher-window"
var bucketName = "\(clientName)_\(clientHostname)"
var excludeTitle = false
var excludeTitlePatterns: [NSRegularExpression] = []

let main = MainThing()
var oldHeartbeat: Heartbeat?
Expand All @@ -140,6 +142,49 @@ encoder.dateEncodingStrategy = .custom({ date, encoder in
start()
RunLoop.main.run()

func compileExcludeTitlePattern(_ pattern: String) -> NSRegularExpression {
do {
return try NSRegularExpression(pattern: pattern, options: [.caseInsensitive])
} catch let regexError {
error("Invalid regex pattern: \(pattern) — \(regexError.localizedDescription)")
exit(1)
}
}
Comment on lines +145 to +152

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P0 Swift's implicit catch binding shadows the error() logging function. In a bare catch { } clause Swift automatically binds the caught value to an implicit constant also named error. Inside this block error is of type Error (not callable), so error("Invalid regex pattern: …") is a compile-time type error and the Swift binary will fail to build. Give the caught value an explicit name to avoid the shadowing.

Suggested change
func compileExcludeTitlePattern(_ pattern: String) -> NSRegularExpression {
do {
return try NSRegularExpression(pattern: pattern, options: [.caseInsensitive])
} catch {
error("Invalid regex pattern: \(pattern)")
exit(1)
}
}
func compileExcludeTitlePattern(_ pattern: String) -> NSRegularExpression {
do {
return try NSRegularExpression(pattern: pattern, options: [.caseInsensitive])
} catch let regexError {
error("Invalid regex pattern: \(pattern)\(regexError.localizedDescription)")
exit(1)
}
}


func parseOptionalArguments(_ arguments: ArraySlice<String>) {
var index = arguments.startIndex
while index < arguments.endIndex {
let argument = arguments[index]

if argument == "--exclude-title" {
excludeTitle = true
index = arguments.index(after: index)
continue
}

if argument == "--exclude-titles" {
let nextIndex = arguments.index(after: index)
guard nextIndex < arguments.endIndex else {
error("Missing value for --exclude-titles")
exit(1)
}
excludeTitlePatterns.append(compileExcludeTitlePattern(arguments[nextIndex]))
index = arguments.index(after: nextIndex)
continue
}

error("Unknown argument: \(argument)")
exit(1)
}
}

func titleShouldBeExcluded(_ title: String) -> Bool {
let range = NSRange(title.startIndex..<title.endIndex, in: title)
return excludeTitlePatterns.contains { pattern in
pattern.firstMatch(in: title, options: [], range: range) != nil
}
}

func start() {
// Arguments should be:
// - url + port
Expand All @@ -148,16 +193,17 @@ func start() {
// - client_id
let arguments = CommandLine.arguments

// Check that we get 4 arguments
if arguments.count != 5 {
print("Usage: aw-watcher-window <url> <bucket> <hostname> <client>")
// Check that we get the 4 required arguments plus any optional flags
if arguments.count < 5 {
print("Usage: aw-watcher-window <url> <bucket> <hostname> <client> [--exclude-title] [--exclude-titles <pattern> ...]")
exit(1)
}

baseurl = arguments[1]
bucketName = arguments[2]
clientHostname = arguments[3]
clientName = arguments[4]
parseOptionalArguments(arguments.dropFirst(5))

guard checkAccess() else {
DispatchQueue.main.asyncAfter(deadline: .now() + 10) {
Expand Down Expand Up @@ -388,6 +434,10 @@ class MainThing {
}
}

if excludeTitle || titleShouldBeExcluded(data.title) {
data.title = "excluded"
}

let heartbeat = Heartbeat(timestamp: nowTime, data: data)
sendHeartbeat(heartbeat)
}
Expand Down
15 changes: 15 additions & 0 deletions aw_watcher_window/macos_cli.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
def build_swift_command(
binpath,
server_address,
bucket_id,
client_hostname,
client_name,
exclude_title=False,
exclude_titles=None,
):
command = [binpath, server_address, bucket_id, client_hostname, client_name]
if exclude_title:
command.append("--exclude-title")
for title in exclude_titles or []:
command.extend(["--exclude-titles", title])
return command
7 changes: 5 additions & 2 deletions aw_watcher_window/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from .config import parse_args
from .exceptions import FatalError
from .lib import get_current_window
from .macos_cli import build_swift_command
from .macos_permissions import background_ensure_permissions

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -80,13 +81,15 @@ def main():

try:
p = subprocess.Popen(
[
build_swift_command(
binpath,
client.server_address,
bucket_id,
client.client_hostname,
client.client_name,
]
exclude_title=args.exclude_title,
exclude_titles=args.exclude_titles,
)
)
# terminate swift process when this process dies
signal.signal(signal.SIGTERM, lambda *_: kill_process(p.pid))
Expand Down
44 changes: 44 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
from aw_watcher_window.macos_cli import build_swift_command


def test_build_swift_command_omits_optional_filters():
command = build_swift_command(
"/tmp/aw-watcher-window-macos",
"http://localhost:5600",
"bucket",
"host.localdomain",
"aw-watcher-window",
)

assert command == [
"/tmp/aw-watcher-window-macos",
"http://localhost:5600",
"bucket",
"host.localdomain",
"aw-watcher-window",
]


def test_build_swift_command_passes_title_filters():
command = build_swift_command(
"/tmp/aw-watcher-window-macos",
"http://localhost:5600",
"bucket",
"host.localdomain",
"aw-watcher-window",
exclude_title=True,
exclude_titles=["Zoom", "Slack.*huddle"],
)

assert command == [
"/tmp/aw-watcher-window-macos",
"http://localhost:5600",
"bucket",
"host.localdomain",
"aw-watcher-window",
"--exclude-title",
"--exclude-titles",
"Zoom",
"--exclude-titles",
"Slack.*huddle",
]
Loading