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: Enable ScratchpadState for sway feature #48

Closed

Conversation

pando85
Copy link

@pando85 pando85 commented Aug 15, 2024

I'm getting an error deserializing Window events in the current version of sway for Arch.

@pando85
Copy link
Author

pando85 commented Aug 15, 2024

If you look for scratchpad_state in https://github.com/swaywm/sway/blob/master/sway/sway-ipc.7.scd you will find it.

@leshow
Copy link
Owner

leshow commented Aug 16, 2024

Did you compile with the sway feature enabled? I'm kind of surprised the deserialize fails, wouldn't it just skip the field but still deserialize successfully?

pasting the error and the version of sway might help. My quick read of the docs doesn't show scratchpad_state in the "Example output" it just says the name will be __i3_scratch.

@pando85
Copy link
Author

pando85 commented Aug 16, 2024

Line 382 in the link.

It fails saying that it cannot be deserialized because that field is missing in the structure.

@leshow
Copy link
Owner

leshow commented Aug 16, 2024

Right, but it should not look for the field if you have the sway feature enabled. It's fine to deserialize a subset of fields.

In any case if it was added in some version of sway I'd like to put that behind a new feature gate so making it required doesn't break old versions of sway.

@pando85
Copy link
Author

pando85 commented Aug 19, 2024

For me it is OK. I'm not in a hurry. I'm planning to migrate to sway but no hurries. So, feel free to modify it to put this under another feature and release whenever you can.

@leshow
Copy link
Owner

leshow commented Aug 25, 2024

Sorry, to reiterate if you have this crate with the sway feature enabled, the field won't attempt to deserialize it will just be skipped, so I'm still kind of confused about how you got an error deserializing a field that shouldn't exist?

I don't have sway so I'm not able to test this, but I will accept a PR if it's properly feature gated

@pando85
Copy link
Author

pando85 commented Aug 25, 2024

Sorry, but that's the problem I guess. It should not be skipped because it is in sway documentation.

I don't know if I'm missing something but that parameter is sent by sway. It is in the documentation (I don't know since which version) and I'm getting this error in my simple program for handling tiling:

RUST_LOG=debug i3-auto-layout
[2024-08-25 10:27:50.116981 +00:00] [ERROR:85] Error receiving window event: Custom { kind: InvalidData, error: Error("missing field `scratchpad_state`", line: 1, column: 744) }
[2024-08-25 10:27:50.117125 +00:00] [DEBUG:90] Sender loop ended
[2024-08-25 10:27:50.117217 +00:00] [DEBUG:99] Receiver loop ended

The branch that I'm executing is this one: https://github.com/pando85/i3-auto-layout/tree/feat/sway-support

That's what I can see. I'm not using sway yet but I will have to migrate to wayland at some point.

@leshow
Copy link
Owner

leshow commented Aug 29, 2024

I can't merge any changes without know which version it changed in to put behind a different feature gate. Someone opened an issue saying the opposite thing here #45 and I don't have sway available to test.

serde doesn't error out when you are parsing a subset of fields, the only way to get that error is if it attempts to get parsed but it isn't there.

That's what I can see. I'm not using sway yet but I will have to migrate to wayland at some point.

how are you testing this without running sway?

@pando85
Copy link
Author

pando85 commented Aug 29, 2024

That log is from a test in sway. I'm not using it as desktop, I was just testing it.

I sent you the documentation and you can run sway but don't worry I don't need you for making this work in my small program.

Thank you for your code, it's working great in i3.

@leshow leshow closed this Aug 30, 2024
@leshow
Copy link
Owner

leshow commented Aug 30, 2024

I'm closing this for now. If someone wants to confirm there is an issue on sway, what version it changed in, then put a feature gate on which fields changed I'll accept that PR.

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.

2 participants