phenax / bsp-layout

Manage layouts in bspwm (tall and wide)
MIT License
373 stars 30 forks source link

A couple of fixes #53

Closed amtoine closed 2 years ago

amtoine commented 2 years ago

This is a duplicate of the stale #32 to apply the requested changed and merge the branch to master.

amtoine commented 2 years ago

i'll address the original requested changes as soon as possible :+1:

amtoine commented 2 years ago

i have to admit i do not get the point of the original snippet below :thinking:

      else
        desk_file="/tmp/bsp-layout_desktop.txt"

        if [ ! -f "$desk_file" ]; then
          # create file
          echo 2 > "$desk_file"
        fi

        if [[ "$event" == "desktop_focus" ]]; then
          if [ "$(< /tmp/bsp-layout_desktop.txt)" == "1" ]; then
            echo 0 > /tmp/bsp-layout_desktop.txt
            __recalculate_layout;
          fi
        else
          if [[ "$(< /tmp/bsp-layout_desktop.txt)" == "0" || "$event" != "desktop_focus" ]]; then
            echo 1 > /tmp/bsp-layout_desktop.txt
            __recalculate_layout;
          fi
        fi
      fi;

"/tmp/bsp-layout_desktop.txt" is mentionned only here in the whole code base. that means that, when entering that else block desk_file would be created for the first time and a 2 would be written to it. however, __recalculate_layout and echo {value} > /tmp/bsp-layout_desktop.txt can be executed only if desk_file contains a 0 or a 1... but, for desk_file to contain a 0 or a 1, it already needs to contain a 1 or a 0, which looks impossible to me here :thinking:

maybe i'm missing something, but i think that this snippet won't ever do something as the desk_file file would get stuck in the 2 state :confused:

even though i don't understand the point of c5ba5e57, i've refactored the snippet a bit in 6eddd6e9 :+1:

EDIT: my bad, it does some things thanks to the || as desk_file is set to 1 as soon as event is not a desktop_focus, e.g. an add_node event :yum:

amtoine commented 2 years ago

for now, i propose to refactor the if statement as follows

        if [[ "$event" == "desktop_focus" && "$(< "$desk_file")" == "1" ]]; then
          echo 0 > "$desk_file"
          __recalculate_layout;
        elif [[ "$event" != "desktop_focus" || "$(< "$desk_file")" == "0" ]]; then
          echo 1 > "$desk_file"
          __recalculate_layout;
        fi

see 99806d09 :+1:

amtoine commented 2 years ago

i'll keep the commit in a separate branch on my fork, in case we need it later :ok_hand:

Available on a2n-s/no-redundant-layout-recalculation :yum: