Skip to content

exec: export matched container id if present #8671

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RobertMueller2
Copy link

@RobertMueller2 RobertMueller2 commented Apr 17, 2025

Provides matched container id to each command execution via handler_context, if --matched-container-id is given with exec
or exec_always. If there is no matched node, the handler_context has the focused node. The idea is that this can simplify scripting.

  • Relates to PR 6160 at i3 (not merged yet). This PR does not have --matched-container-id. After a while I decided for it because it felt better. thought process
  • I don't think this enables anything that wasn't possible before.
  • I decided against a function and for inline placement for no particular reason.
    • Since I made it inline though, goto seemed like a cleaner choice than guards or nesting more ifs
  • I looked at mark.c for inspiration for multi argument handling. This could potentially do with redundancy reduction now, but that may be valid for even more command files. I did not feel comfortable addressing these things with this simple PR.
  • I may accidentally have fixed a bug while doing this in line 35 (old). The below is what I observe before patch for using --no-startup-id and then triggering an error due to no further args -- I believe this should say "exec" or "exec_always", not "--no-startup-id".
> swaymsg -- exec --no-startup-id
Error: Invalid --no-startup-id command (expected at least 1 argument, got 0)
  • this is now
Error: Invalid exec command (expected at least 1 argument, got 0)

and

Error: Invalid exec_always command (expected at least 1 argument, got 0)

for the tests below.

I'm using this change in my regular sway session.

Tested with:

#!/bin/sh

i=1

echo "20 tests"
echo

for exec_variant in "exec" "exec_always" ; do
    for command_parameter in 'env | grep "\(DESKTOP_STARTUP_ID\|SWAY_EXEC_CON_ID\)"' '' ; do
        for mode in 0 1 2 3 7; do
            HAVE_NODS=
            HAVE_CONID=
            NO_CONID=
            # 1 = DESKTOP_STARTUP_ID in front, 4 = DESKTOP_STARTUP_ID in the back
            if [ $(($mode & 0x1)) -eq $((0x1)) -o $(($mode & 0x4)) -eq $((0x4)) ];then
                HAVE_NODS=1
            fi

            if [ $(($mode & 0x2)) -eq $((0x2)) ];then
                HAVE_CONID=1
            else
                NO_CONID=1
            fi

            if [ -n "$command_parameter" ];then
                swaymsg -- exec "echo \"test ${i}: mode: ${mode}, exec_variant: ${exec_variant}, command_parameter: ${command_parameter:+set}\n---\nexpect: DESKTOP_STARTUP_ID ${HAVE_NODS:+not }set, SWAY_EXEC_CON_ID ${NO_CONID:+not }set\""
            else
                swaymsg -- exec "echo \"test ${i}: mode=${mode}, exec_variant=${exec_variant}, command_parameter ${command_parameter:+set}\n---\nexpect: error message in terminal\""
            fi

            sleep .5
            swaymsg -- exec "echo \"actual command: ${exec_variant}${HAVE_NODS:+ --no-startup-id}${HAVE_CONID:+ --matched-container-id}${command_parameter:+ }$command_parameter\""
            sleep .5
            swaymsg -- ${exec_variant}${HAVE_NODS:+ --no-startup-id}${HAVE_CONID:+ --matched-container-id}${command_parameter:+ }$command_parameter
            sleep .5
            swaymsg -- exec 'echo "---"'
            i=$((i+1))
            read -p "press key to continue..." x
        done
    done
done

My demo use case:

[title="Picture-in-Picture"] floating enable, sticky enable, mark --add _newstickychange, mark --add _newnotitle, resize set 20ppt 20ppt, exec swayhelper move_window_to_corner 3 --title Picture-in-Picture

can now become

[title="Picture-in-Picture"] floating enable, sticky enable, mark --add _newstickychange, mark --add _newnotitle, resize set 20ppt 20ppt, exec --matched-container-id swayhelper move_window_to_corner 3

While the former would even likely work without any matching in my swayhelper, just working on focused node, it is not guaranteed if this was matching several windows, in that case I have to duplicate the criteria as shown above. The latter is not shorter depending on the actual criteria, but has less redundancy.

@RobertMueller2 RobertMueller2 force-pushed the exec-env branch 4 times, most recently from 77b11fd to 84c97ae Compare April 18, 2025 11:08
@RobertMueller2
Copy link
Author

RobertMueller2 commented Apr 22, 2025

As I was optimising some of my scripts my eye got caught by SWAY_EXEC_CON_ID in a terminal window. Of course I was aware before the patch does this, it's the whole point of it. But just seeing the var while doing something entirely different triggered some new thought process, I guess.

If I start an application from the terminal, it'll have this env var from the terminal. I don't think that has consequences, but it would be an environment with an ID from another window, and that does not feel right.

Secondly, while I'm not sure if I can construct a case where the variable is "too much information" (e.g. in sandboxing scenarios I'd always make sure the environment is clean), only exporting it on purpose in select scenarios feels better than doing it always.

So I think I'll make this a draft again and try and add a flag for exec.

EDIT: all fixed.

@RobertMueller2 RobertMueller2 marked this pull request as draft April 22, 2025 10:17
@RobertMueller2 RobertMueller2 marked this pull request as ready for review April 22, 2025 22:40
Provides matched container id to each command execution via
handler_context, if --matched-container-id is given with exec
or exec_always. If there is no matched node, the handler_context has
the focused node. The idea is that this can simplify scripting.

Relates to PR 6160 at i3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant