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(detect): update sctk dependency to fix crash #1769

Merged
merged 2 commits into from Dec 10, 2023

Conversation

morgan-dgk
Copy link
Collaborator

Integrates the fix from smithay-client-toolkit PR 406.

This should resolve #1768 and #1750.

Testing?

Compiled with patched smithay-client-toolkit package and confirmed espanso runs normally under Hyprland v0.33.1.

Anything Else?

This is my very first PR, apologies if information is missing.

I am unsure how the smithay-client-toolkit dependency should ultimately be patched. Currently Cargo.toml points to the fork I made of sctk. I tried naively updating to 16.1 which should include this fix, but there are breaking changes which cause the build to fail.

Thanks!

@federico-terzi
Copy link
Collaborator

Hey @morgan-dgk, thanks for the investigation! I think we should prefer bumping the smithay-client-toolkit version rather than using a patched version from git, as the latter might become tricky to maintain.

If you can update this PR updating the version to 16.1, I'd be happy to help you fix the build errors :)

PS: please pull from dev to access the latest fixes

@morgan-dgk
Copy link
Collaborator Author

Thanks for your guidance @federico-terzi, I have updated my PR to bump sctk to 16.1. Build is failing due to a type error in espanso-detect/src/evdev/sync/wayland:

85 |           match map_keyboard_repeat(
   |  _______________-
86 | |           event_loop.handle(),
87 | |           &seat,
88 | |           None,
89 | |           RepeatKind::System,
90 | |           move |event, _, _| keyboard_event_handler(event, &result_clone),
91 | |         ) {
   | |_________- this expression has type `Result<WlKeyboard, smithay_client_toolkit::seat::keyboard::Error>`
92 |             Ok((kbd, repeat_source)) => {
   |                ^^^^^^^^^^^^^^^^^^^^ expected `WlKeyboard`, found `(_, _)`
   |
   = note: expected struct `WlKeyboard`
               found tuple `(_, _)`

@federico-terzi
Copy link
Collaborator

@morgan-dgk Thanks! Looking at this example, it seems they removed the second parameter of the call

So we'd need to:

  • Change the seats type to be
let mut seats = Vec::<(String, Option<wl_keyboard::WlKeyboard>)>::new();

And have the match statements to be:

 Ok(kbd) => {
                        seats.push((name, Some(kbd)));
                    }

(we basically don't use the second parameter anymore)

Do you think you could give it a spin? :)

@morgan-dgk
Copy link
Collaborator Author

@federico-terzi thanks for your encouragement :). I gave it a shot - appears to build locally. I'm not sure if additional changes are needed in the match statement on map_keyboard_repeat.

@federico-terzi
Copy link
Collaborator

Looks great! Let's see if the CI is happy

@federico-terzi
Copy link
Collaborator

All green! Before merging, did you check if it's working correctly locally? I mean, giving it a run on Wayland :)

@morgan-dgk
Copy link
Collaborator Author

Just did, matching and replacment works fine on Hyprland v0.33.1. However, it looks like something may have broken with the UI rendering though as the popup window no longer shows Espanso logo or text, just a rainbow gradient:

image

@federico-terzi
Copy link
Collaborator

Oh interesting! I guess that's fine for now, it's better for it to work than for it to be beautiful :)

Thanks for the effort!

@federico-terzi federico-terzi changed the title update sctk dependency to point to patched version fix(detect): update sctk dependency to fix crash Dec 10, 2023
@federico-terzi federico-terzi merged commit 655ebb1 into espanso:dev Dec 10, 2023
9 checks passed
federico-terzi added a commit that referenced this pull request Dec 17, 2023
* fix: clippy warnings (#1426)

* fix: bad window style that caused Search bar to crash on macOS ventura #1413 and missed initialization that caused segmentation fault on some cases (#1424)

* fix(detect): update sctk version to fix #1057 on wayland

Co-authored-by: Ricky Kresslein <rk@lakoliu.com>

* chore: bump version

* chore(deps): bump regex from 1.4.6 to 1.5.5 (#1428)

Bumps [regex](https://github.com/rust-lang/regex) from 1.4.6 to 1.5.5.
- [Release notes](https://github.com/rust-lang/regex/releases)
- [Changelog](https://github.com/rust-lang/regex/blob/master/CHANGELOG.md)
- [Commits](rust-lang/regex@1.4.6...1.5.5)

---
updated-dependencies:
- dependency-name: regex
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: version bump

* fix: clippy warnings (#1487)

* fix: clippy warnings

* fix: clippy warning

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: clippy warnings

* fix: cleanup and crate updates (#1602)

* update incompatible packages

* fix some clippy warnings

* fix clippy warnings

* update winrt-notification version

* fix formatting

* fix warning

* fix warning

* fix: upgrade Rust version in Linux release pipeline

* Minor improvements to README (#1581)

- Remove trailing spaces
- Remove inline HTML

* fix: compilation issues and warnings (#1770)

* fix some warnings

* fix issue

* fix: bump rust version on Linux pipelines

* chore: bump cargo-deb dependency

* chore: refactor CI deployment pipelines (#1771)

* chore: refactor CI deployment pipelines

* rollback test change

* chore(deps): bump webpki from 0.22.0 to 0.22.2 (#1772)

Bumps [webpki](https://github.com/briansmith/webpki) from 0.22.0 to 0.22.2.
- [Commits](https://github.com/briansmith/webpki/commits)

---
updated-dependencies:
- dependency-name: webpki
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat(core): enable alt-code emulation by default on Windows (#1603)

* fix(config): fix warning

* fix(config): fix warning

* chore(deps): bump openssl from 0.10.36 to 0.10.61 (#1773)

Bumps [openssl](https://github.com/sfackler/rust-openssl) from 0.10.36 to 0.10.61.
- [Release notes](https://github.com/sfackler/rust-openssl/releases)
- [Commits](sfackler/rust-openssl@openssl-v0.10.36...openssl-v0.10.61)

---
updated-dependencies:
- dependency-name: openssl
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* feat: update documentation (#1774)

* fix(detect): update sctk dependency to fix crash (#1769)

* bump sctk to 16.1

* Update seats type and match statements to match sctk v16

* Improve code quality opting-in some `clippy` pedantic lints (#1775)

* 🚨 Change single points of failure for `assert!`

* 🐛 Fix a missing `;`

* 🚨 Add missing `;` for consistent formatting

* 🚨 Fix `{}` with `{variable}` lint

* 🚨 Remove redundant else blocks

* 🚨 Separate numbers with `_` inbetween

* 🚨 Fix unused `&self` argument

* 🚨 Fix arg passed by value but not consumed-part 1

* 🚧 Trying to compile linux build

* Revert "🚨 Fix arg passed by value but not consumed-part 1"

This reverts commit 7c54210.

* Revert ":construction: Trying to compile linux build"

This reverts commit 617361d.

* 🚨 Change redundant closures with std methods

* 🚨 Change `for_each()` for `for` cycles

* 🚨 Add 2 more of `panic` in `if` statements

* 🚨 Change negative if for positive if

* 🚨 Fix unnested `or` patterns

* 🚨 Add some more `;`

* 🚨 Rename `_var` to `var` when it's possible

* 🚨 Remove `iter()` in for cycles

* 🐛 Fix `keys()` bug

* 🚨 Fix `"".to_string()` into `String::new()`

* 🚨 Fix `""to_owned()` into `String::new()`

* 🚨 Fix clippy and derive `Default` trait

* 🚨 Rename `_var` for `var` some more

* 🚨 Another block of adding `;`

* 🐛 Fix `_` unused var that breaks clippy

* 🚨 Fix _some_ star imports

* 🚨 Remove unnecessary use of `vec![]`

* chore: 📝 Update the path for windows portable compilation (#1782)

* feat(core): add label to espanso cli on match (#1720)

* refactor(match_cli): update print_matches functions to include label and handle errors

* fix formatting

* fix types

* fix optionality

---------

Co-authored-by: Federico Terzi <federico-terzi@users.noreply.github.com>

* fix(core): update package dependencies for Debian 12+ and Ubuntu 22.10+ (#1697)

Removes '$auto' keyword to support dependencies

Fixes #1674

Fixes #1662

* docs: improve description of compilation on Windows (#1785)

* chore: 🚨 more `clippy` pedantic lints (#1779)

* 🚨 Fix 1 case of 1 if for 1 panic (do `assert!`)

* 🚨 Remove unnecessary `mut`

* 🚨 Fix `{}` with `{variable}` lint

* 🚨 Fix redundant closure

* 🚨 Fix `map(<f>).unwrap_or_else(<g>)`

* 🚨 Rewrite to `let...else`

* 🚨 Remove unnecessary hashes `#` in raw strings

* 🚨 Remove useless `for_each`

* 🚨 De-reference double referenced values (`&&`)

* 🚨 Add ` to documentation

* 🚨 A couple of lints

* 🚨 Replace `match` with 1 arm for `if let`

* 🚨 Change `Default` for `Struct::default()`

* 🚨 Change `Ok(_)` with `Ok(())`

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: lakoliu <99976966+lakoliu@users.noreply.github.com>
Co-authored-by: Ricky Kresslein <rk@lakoliu.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ashish Bhatia <ashishb@ashishb.net>
Co-authored-by: Morgan <morgan.k@skiff.com>
Co-authored-by: Auca Coyan <aucacoyan@gmail.com>
Co-authored-by: Erwann Mest <m+github@kud.io>
Co-authored-by: Rick Calixte <10281587+rcalixte@users.noreply.github.com>
Co-authored-by: Nai Hao Cheng <chengnaihao@gmail.com>
AucaCoyan added a commit to AucaCoyan/espanso that referenced this pull request Feb 13, 2024
AucaCoyan added a commit to AucaCoyan/espanso that referenced this pull request Feb 13, 2024
AucaCoyan added a commit to AucaCoyan/espanso that referenced this pull request Feb 13, 2024
@morgan-dgk morgan-dgk deleted the sctk-wayland-fix branch March 15, 2024 18:48
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.

Espanso crashes under Hyprland v0.32.3 and above
2 participants