phenax / bsp-layout

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

Unnecessary dependencies #70

Closed olafdietsche closed 1 year ago

olafdietsche commented 1 year ago

What did you expect to happen?

There should be no dependency to bc, because it isn't used anywhere in the scripts.

What actually happened?

https://github.com/phenax/bsp-layout/blob/master/src/utils/common.sh aborts, if bc isn't installed.

Describe your attempts to resolve the issue

Nothing done so far.

Steps to reproduce

  1. Uninstall bc or rename bc, e.g. mv /usr/bin/bc /usr/bin/bc.off
  2. Run bsp-layout
  3. bsp-layout aborts with "[Missing dependency] bsp-layout needs bc installed"
  4. Don't forget to reinstall bc or revert the renaming: mv /usr/bin/bc.off /usr/bin/bc

System Information

I installed the bsp-layout command with make DESTDIR=/tmp/destdir install.

$ uname -osrm
Linux 5.15.0-56-generic x86_64 GNU/Linux
$ /tmp/destdir/usr/local/bin/bsp-layout version
0.0.10-1
olafdietsche commented 1 year ago

Still playing with bsp-layout. When doing bsp-layout set tall everything looks ok. But when I say bsp-layout once tall, it shows an error

/tmp/destdir/usr/local/lib/bsp-layout/layouts/tall.sh: line 53: 0.6 * 1920 : syntax error: invalid arithmetic operator (error token is ".6 * 1920 ")

But this is with bc available, so maybe this issue (#70) is invalid and there is another bug.

amtoine commented 1 year ago

hello @olafdietsche :wave:

it appears bc is not used anywhere, good catch :ok_hand:

Still playing with bsp-layout. When doing bsp-layout set tall everything looks ok. But when I say bsp-layout once tall, it shows an error

/tmp/destdir/usr/local/lib/bsp-layout/layouts/tall.sh: line 53: 0.6 * 1920 : syntax error: invalid arithmetic operator (error token is ".6 * 1920 ")

But this is with bc available, so maybe this issue (#70) is invalid and there is another bug.

i am able to reproduce this, without bc even installed on my machine and with the dependency removed :thinking:

could you file another issue about this bug? :relieved:

olafdietsche commented 1 year ago

According to bash - Shell Arithmetic and various online sources, bash doesn't handle floating point by itself. So maybe this is the reason for the bc dependency, even though it isn't used (yet).

The other issue is at https://github.com/phenax/bsp-layout/blob/master/src/layouts/tall.sh#L53:

local want=$(( $master_size * $mon_width ))

and occurs, because of $master_size being 0.6. I haven't figured out, why this happens with once only.

olafdietsche commented 1 year ago

It looks like commit https://github.com/phenax/bsp-layout/commit/feae0882dcad23112e3acba81c3118fc4b53f3eb introduced this.

There is a change replacing bc with $(( ... )). This is ok as long as there's only integer arithmetic. But since there's floating point involved here, the change isn't valid. So maybe review this commit again for other possible issues.

olafdietsche commented 1 year ago

So, finally this issue is invalid and is the result of the mentioned commit. Therefore I close the issue.

amtoine commented 1 year ago

So, finally this issue is invalid and is the result of the mentioned commit. Therefore I close the issue.

if you still have an issue, even though it's not relevant in this very issue and comes from another commit, you could file a new issue :yum: