Problem
The logic for detecting the "Tomorrow" list name in detect_list_names() is unnecessarily complex and hard to read:
# things_jxa.py:87-94
for locale, tomorrow_name in known_tomorrow.items():
if locale in LOCALE_MAPPINGS:
first_list = detected_mapping.get("inbox", "")
# If we detected Inbox correctly, use the same locale for Tomorrow
if first_list == LOCALE_MAPPINGS[locale]["inbox"]:
detected_mapping["tomorrow"] = LOCALE_MAPPINGS[locale]["tomorrow"]
break
Issues:
- Triple-nested logic (for → if → if)
- Indirect approach: checks if inbox matches to infer locale
- Variable name
first_list is misleading (it's specifically the inbox)
- Could be simplified with a reverse lookup
Current Implementation
Location: things_jxa.py:79-98
The function:
- Iterates through known translations of "Tomorrow"
- Checks if locale exists in LOCALE_MAPPINGS
- Compares detected inbox name to known inbox names
- If match found, uses that locale's "Tomorrow" translation
Proposed Solution
Use a reverse mapping for cleaner logic:
# Build reverse mapping once at module level
REVERSE_LOCALE_MAP = {}
for locale, mappings in LOCALE_MAPPINGS.items():
for key, value in mappings.items():
if value not in REVERSE_LOCALE_MAP:
REVERSE_LOCALE_MAP[value] = {}
REVERSE_LOCALE_MAP[value][key] = locale
def detect_list_names():
# ... existing detection ...
# Detect locale from inbox name
inbox_name = detected_mapping.get("inbox")
if inbox_name and inbox_name in REVERSE_LOCALE_MAP:
detected_locale = REVERSE_LOCALE_MAP[inbox_name].get("inbox")
if detected_locale and detected_locale in LOCALE_MAPPINGS:
detected_mapping["tomorrow"] = LOCALE_MAPPINGS[detected_locale]["tomorrow"]
Or even simpler:
def detect_locale_from_inbox(inbox_name: str) -> Optional[str]:
"""Return locale code if inbox name matches a known locale."""
for locale, mappings in LOCALE_MAPPINGS.items():
if mappings.get("inbox") == inbox_name:
return locale
return None
# In detect_list_names():
inbox_name = detected_mapping.get("inbox")
detected_locale = detect_locale_from_inbox(inbox_name)
if detected_locale:
detected_mapping["tomorrow"] = LOCALE_MAPPINGS[detected_locale]["tomorrow"]
Impact
Priority: Medium - code quality improvement
Affected code: things_jxa.py:79-98
Breaking changes: None (internal refactoring)
Benefits:
- Improved readability
- Easier to test
- Reusable locale detection logic
Acceptance Criteria
Additional Context
This issue was identified during post-implementation review of #1 (multi-locale support).
Problem
The logic for detecting the "Tomorrow" list name in
detect_list_names()is unnecessarily complex and hard to read:Issues:
first_listis misleading (it's specifically the inbox)Current Implementation
Location:
things_jxa.py:79-98The function:
Proposed Solution
Use a reverse mapping for cleaner logic:
Or even simpler:
Impact
Priority: Medium - code quality improvement
Affected code:
things_jxa.py:79-98Breaking changes: None (internal refactoring)
Benefits:
Acceptance Criteria
Additional Context
This issue was identified during post-implementation review of #1 (multi-locale support).