Skip to content

Conversation

@cachebag
Copy link
Contributor

@cachebag cachebag commented Jan 22, 2026

This PR implements connection::Builder::ibus() for connecting with IBus. The IBus address is fetched from the ibus address command, and is handled similarly to how we handle the launchd address on macOS.

Fixes #964

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 22, 2026

Merging this PR will not alter performance

✅ 22 untouched benchmarks


Comparing cachebag:main (591cb02) with main (7ffc273)

Open in CodSpeed

Copy link
Contributor

@zeenix zeenix left a comment

Choose a reason for hiding this comment

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

Didn't look very thoroughly yet but looks good to me in general.

use crate::{Address, Result, process::run};

#[derive(Clone, Debug, PartialEq, Eq)]
#[non_exhaustive]
Copy link
Contributor

Choose a reason for hiding this comment

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

I dind't know you can use this attribute on a struct. Does it make it so that people can't assume the fields? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep and the user must use the provided constructor but thinking about this more, I think I will just remove it..it's a zero-sized unit struct with no fields, so there's nothing to be "exhaustive" about. I think I just instintively matched launchd's struct (which actually has fields)

///
/// # Example
///
/// ```no_run
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should run the test, if possible. We can make it linux-specific and install/launch ibus in the linux test job in the CI (or we can mock the server side too, if we want it to be reliable and runable anywhere).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How would we integrate this into CI? Just getting the deps for ibus sounds like a pain unless you're familiar with methods to do so

Copy link
Contributor

@zeenix zeenix Jan 25, 2026

Choose a reason for hiding this comment

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

Just getting the deps for ibus sounds like a pain unless you're familiar with methods to do so

apt-get install package-names.. like we already do? 😉

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whattt lol. I didn't even know you were running dbus in CI. I've been trying to get NetworkManager to run in my CI forever..though maybe they don't work the same since nm needs a network device..

@cachebag cachebag marked this pull request as draft January 25, 2026 06:55
Implement `connection::Builder::ibus()` for connecting with IBus. The IBus
address is fetched from the `ibus address` command, handled similarly to
how we handle the `launchd` address on macOS.

Fixes z-galaxy#964
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.

Helper for ibus connection creation

2 participants