Skip to content

kill subscriber process when desktop is removed #60

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.

Sign up for GitHub

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
amtoine wants to merge 4 commits into
base: master
Choose a base branch
from
Open

kill subscriber process when desktop is removed #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6a72d6
kill subscriber process when desktop is removed
no3m Oct 19, 2021
b38e145
add the utils.desktop.get_desktop_id tool function
amtoine Aug 12, 2022
3186fc7
separate the layout and process killing
amtoine Aug 12, 2022
1d68ece
fix a bad typo in the kill_layout function
amtoine Aug 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 24 additions & 4 deletions src/layout.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,24 @@ kill_layout() {
kill $old_pid 2> /dev/null || true;
}

# Kill old process
kill_process() {
local old_pid=$1;
kill $old_pid 2> /dev/null || true;
}

remove_listener() {
desktop=$1;
[[ -z "$desktop" ]] && desktop=$(get_focused_desktop);

kill_layout "$desktop";
local old_pid="$(get_desktop_options "$desktop" | valueof pid)";
Copy link
Owner

@phenax phenax Aug 18, 2022

Choose a reason for hiding this comment

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

I don't get the point of this change. It was killing the process before resetting the state, now it does that after resetting the state but what does that accomplish?

Copy link
Collaborator Author

@amtoine amtoine Aug 18, 2022

Choose a reason for hiding this comment

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

that is a change from the original PR, not sure what the goal was here to be honest 🤔

and, with the changes you originally requested in #36, kill_layout "$desktop" and

old_pid="$(get_desktop_options "$desktop" | valueof pid)"
kill_process "$old_pid"

should do exactly the same thing 👍

Copy link
Collaborator Author

@amtoine amtoine Aug 18, 2022

Choose a reason for hiding this comment

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

change the way kill_layout gets called... desktop options in remove_listener have to be cleared before calling kill_layout,
otherwise if remove_listener is called by the subscriber process on desktop_remove event, it will kill itself before the options are cleared.

that is the original comment on the PR 👌


# Reset process id and layout
set_desktop_option $desktop 'layout' "";
set_desktop_option $desktop 'pid' "";
set_desktop_option $desktop 'did' "";

kill_layout $old_pid;
}

get_layout_file() {
Expand Down Expand Up @@ -141,13 +150,21 @@ start_listener() {
__recalculate_layout() { run_layout $layout $args 2> /dev/null || true; }

# Then listen to node changes and recalculate as required
bspc subscribe node_{add,remove,transfer,flag,state} desktop_focus | while read line; do
bspc subscribe node_{add,remove,transfer,flag,state} desktop_focus desktop_remove | while read line; do
event=$(echo "$line" | awk '{print $1}');
arg_index=$([[ "$event" == "node_transfer" ]] && echo "6" || echo "3");
desktop_id=$(echo "$line" | awk "{print \$$arg_index}");
desktop_name=$(get_desktop_name_from_id "$desktop_id");

if [[ "$desktop_name" = "$selected_desktop" ]]; then
if [[ "$event" == "desktop_remove" ]]; then
selected_desktop_id=$(get_desktop_options "$selected_desktop" | valueof did);
if [[ "$desktop_id" == "$selected_desktop_id" ]]; then
remove_listener "$selected_desktop";
exit 0;
fi
fi

if [[ "$desktop_name" == "$selected_desktop" ]]; then
__initialize_layout;

if [[ "$event" == "node_transfer" ]]; then
Expand All @@ -165,11 +182,14 @@ start_listener() {
disown;

# Kill old layout
kill_layout $selected_desktop;
local old_pid="$(get_desktop_options "$selected_desktop" | valueof pid)";
kill_layout $old_pid;

desktop_id=$(get_desktop_id "$selected_desktop");
# Set current layout
set_desktop_option $selected_desktop 'layout' "$layout";
set_desktop_option $selected_desktop 'pid' "$LAYOUT_PID";
set_desktop_option $selected_desktop 'did' "$desktop_id";

# Recalculate styles as soon as they are set if it is on the selected desktop
if [[ "$(get_focused_desktop)" == "$selected_desktop" ]]; then
Expand Down
2 changes: 1 addition & 1 deletion src/utils/desktop.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@ names="--names"
[[ $USE_NAMES -eq 0 ]] && names="";
get_focused_desktop() { bspc query -D -d 'focused' $names; }
get_desktop_name_from_id() { bspc query -D -d "$1" $names; }

get_desktop_id() { bspc query -D -d "$1"; }