Skip to content
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

Fix null monitors causing panics #302

Merged

Conversation

zjeffer
Copy link
Contributor

@zjeffer zjeffer commented Jan 7, 2025

Basically what @chmanie mentioned here

I haven't used hyprland-rs yet so some testing is needed by people with actual experience ;)

@zjeffer zjeffer force-pushed the fix/zjeffer/monitor-id-option branch from 2af977c to 9e7d409 Compare January 12, 2025 11:26
@chmanie
Copy link

chmanie commented Jan 12, 2025

Ok, so I tested this with ironbar and there's one more problem with event parsing. Specifically here:

ParsedEventType::MonitorAddedV2 => Ok(Event::MonitorAdded(MonitorAddedEventData {
id: parse_int!(get![ref args;0], event: "MonitorAddedV2" => u8),
name: get![args;1],
description: get![args;2],
})),

I'm getting this error in the event listener:

Internal("MonitorAddedV2: invalid integer error: invalid digit found in string")

I tested this and the id is actually -1. Making it an i8 here

and here

id: parse_int!(get![ref args;0], event: "MonitorAddedV2" => u8),

instead of a u8 fixes this and also fixes the problem in ironbar.

@yavko
Copy link
Member

yavko commented Jan 12, 2025

Ok, so I tested this with ironbar and there's one more problem with event parsing. Specifically here:

ParsedEventType::MonitorAddedV2 => Ok(Event::MonitorAdded(MonitorAddedEventData {
id: parse_int!(get![ref args;0], event: "MonitorAddedV2" => u8),
name: get![args;1],
description: get![args;2],
})),

I'm getting this error in the event listener:

Internal("MonitorAddedV2: invalid integer error: invalid digit found in string")

I tested this and the id is actually -1. Making it an i8 here

and here

id: parse_int!(get![ref args;0], event: "MonitorAddedV2" => u8),

instead of a u8 fixes this and also fixes the problem in ironbar.

If that's the case it should just be a MonitorId type

@yavko
Copy link
Member

yavko commented Jan 12, 2025

Can you guys test, if it works I'll merge

@chmanie
Copy link

chmanie commented Jan 12, 2025

Tested with MonitorId (i128); works as well.

Copy link
Member

@yavko yavko left a comment

Choose a reason for hiding this comment

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

lgtm

@yavko yavko merged commit bc428ac into hyprland-community:master Jan 13, 2025
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.

3 participants