sway + input #40

Open
adriangl wants to merge 6 commits from syncope into main
Owner
No description provided.
adriangl added 5 commits 2025-02-09 00:11:35 +01:00
Author
Owner

Feel free to nit

Feel free to nit
adriangl added 1 commit 2025-02-10 10:20:30 +01:00
oysteikt reviewed 2025-02-27 09:16:46 +01:00
@ -10,2 +10,4 @@
[dependencies]
regex = "1.5.4"
lazy_static = "1.4.0"
Owner
See https://github.com/rust-lang-nursery/lazy-static.rs/issues/214
@ -22,0 +22,4 @@
Custom api call in curl examples
LOL with input command. (utilizing ydotools)
Owner

It's not really clear to me what this does. Could you expand it a bit?

It's not really clear to me what this does. Could you expand it a bit?
@ -14,0 +14,4 @@
enableFirefox = lib.mkEnableOption "include fiorefox browser for external urls";
enableInput = lib.mkEnableOption "enable input maipulation with ydotool,";
Owner

mkEnableOption wraps your string in Whether to enable ${str}.. That would leave this as Whether to enable enable input.... Also, there's a comma at the end

`mkEnableOption` wraps your string in `Whether to enable ${str}.`. That would leave this as `Whether to enable enable input...`. Also, there's a comma at the end
@ -176,6 +195,7 @@ in
users.greg = {
isNormalUser = true;
group = "greg";
extraGroups [ "audio" "video" "input" "ydotool" ];
Owner

Is ydotool a group?

Is ydotool a group?
@ -171,2 +171,4 @@
.map_err(|e| e.into())
}
// pub async fn sway_run_command(command: String) -> anyhow::Result<()> {
Owner

Please remove leftover comments

Please remove leftover comments
@ -173,0 +188,4 @@
}
pub async fn sway_get_workspace_names() -> anyhow::Result<Vec<String>> {
tokio::task::spawn_blocking(|| -> anyhow::Result<Vec<String>> {
Owner

None of these seem to be async. Maybe they can just be run as normal, without spawn_blocking?

None of these seem to be async. Maybe they can just be run as normal, without `spawn_blocking`?
@ -173,0 +237,4 @@
// connection.run_command(&format!("exec firefox --kiosk {}", url))?; //moved to firefox to pin in kiosk mode. potentially add --new-window
//get the DEFAULT_BROWSER env var
let default_browser = std::env::var("DEFAULT_BROWSER").unwrap_or("firefox".to_string());
Owner

This kinda feels like a security hole waiting to happen. Could we bake in the executable path during build time instead maybe?

This kinda feels like a security hole waiting to happen. Could we bake in the executable path during build time instead maybe?
@ -173,0 +248,4 @@
pub async fn sway_close_workspace(workspace: String) -> anyhow::Result<()> {
tokio::task::spawn_blocking(move || -> anyhow::Result<()> {
let mut connection = swayipc::Connection::new()?;
Owner

I wonder if it would be better to let these api functions take in a swayipc connection, so we don't have to spawn a new one all the time. Do you know roughly how much overhead this brings?

I wonder if it would be better to let these api functions take in a swayipc connection, so we don't have to spawn a new one all the time. Do you know roughly how much overhead this brings?
@ -52,3 +52,3 @@
.split_for_parts();
router.merge(SwaggerUi::new("/docs").url("/docs/openapi.json", api))
router.merge(SwaggerUi::new("/docs/v1").url("/docs/v1/openapi.json", api))
Owner

Remind me to have a look at this

Remind me to have a look at this
This pull request has changes conflicting with the target branch.
  • Cargo.lock
  • src/main.rs
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin syncope:syncope
git checkout syncope
Sign in to join this conversation.
No Reviewers
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Grzegorz/greg-ng#40
No description provided.