Skip to content

Conversation

@DhruvaSambrani
Copy link

Adds --format and --json to list alarms. Semi-fixes #148, actual alarm implementation can be done externally.

@DhruvaSambrani DhruvaSambrani marked this pull request as ready for review October 17, 2025 23:34
Copy link
Contributor

@mathstuf mathstuf left a comment

Choose a reason for hiding this comment

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

Code looks reasonable to me; needs some pre-commit love though.

@DhruvaSambrani
Copy link
Author

I am unable to see the failure of pre-commit.ci, I only see Internal Service error sent as json (??). Is this temporary or am I missing smthing?

@mathstuf
Copy link
Contributor

Interesting; it works with uBlock Origin for me; maybe something is interfering with full networking? Anyways, here is the output:

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check toml...............................................................Passed
check for added large files..............................................Passed
debug statements (python)................................................Passed
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

khal/khalendar/event.py:725: error: Incompatible types in assignment (expression has type "list[dict[str, object]]", target has type "str")  [assignment]
khal/controllers.py:329: note: By default the bodies of untyped functions are not checked, consider using --check-untyped-defs  [annotation-unchecked]
Found 1 error in 1 file (checked 57 source files)

ruff (legacy alias)......................................................Failed
- hook id: ruff
- exit code: 1

E501 Line too long (101 > 100)
   --> khal/utils.py:202:101
    |
200 |             if 'alarms-list' in row and isinstance(row['alarms-list'], list):
201 |                 row['alarms-list'] = ", ".join(
202 |                     alarm['description']+"@"+alarm['delta-formatted'] for alarm in row['alarms-list']
    |                                                                                                     ^
203 |                 )
    |

E501 Line too long (166 > 100)
   --> tests/cli_test.py:498:101
    |
496 | …
497 | …
498 | …"", "delta-formatted": "-15m"}, {"delta": -3600.0, "description": "", "delta-formatted": "-1h"}]}]'
    |                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
499 | …
500 | …
    |

Found 2 errors.

@WhyNotHugo
Copy link
Member

@DhruvaSambrani sounds like a temporary glitch. You can also run these checks with pre-commit run locally.

@DhruvaSambrani
Copy link
Author

DhruvaSambrani commented Oct 20, 2025

I can fix it either by typing the attributes dict explicitly, or serialize json to keep the same type.

If I change the type to dict[str, Union[str, list[dict[str, object]]]], it adds an error on line 694

khal/khalendar/event.py:694: error: No overload variant of "__add__" of "list" matches argument type "str"  [operator]
khal/khalendar/event.py:694: note: Possible overload variants:
khal/khalendar/event.py:694: note:     def __add__(self, list[dict[str, object]], /) -> list[dict[str, object]]
khal/khalendar/event.py:694: note:     def [_S] __add__(self, list[_S], /) -> list[_S | dict[str, object]]
khal/khalendar/event.py:694: note: Left operand is of type "str | list[dict[str, object]]"
khal/khalendar/event.py:694: error: Unsupported operand types for + ("str" and "list[dict[str, object]]")  [operator]
khal/khalendar/event.py:694: note: Both left and right operands are unions

This can also be fixed by typing it as dict[str, Any], but maybe that is too generically typed?

The other way is to use a TypedDict, but that seems too complicated for a pre-ci fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Alarm user on alarm

3 participants