[i3ass][i3fyra][i3Kornhe] i3Kornhe also accepts any one argument as last argument, as long as it starts with `u|l|d|r|up|left|down|right` #195

New issue

Closed

opened 2023-07-20 13:52:52 +00:00 by 1ntronaut · 1 comment

1ntronaut commented

2023-07-20 13:52:52 +00:00

(Migrated from github.com)

Description
A clear and concise description of what the bug is.

1. New issue: looking at my i3config, I seem to use i3viswiz to focus; and only seem to use i3Kornhe move to actually MOVE my windows across containers. But: it does also exhibit some of the same behaviour.
3.5 NOTE: i3Kornhe seems to work fine in conjunction with i3fyra: it does (after populating my ABCD containers) correctly move/hide/show the ABCD containers as expected from i3fyra (as you can also see in the output below)

To Reproduce

see below

this entry focuses up:

~ > i3Kornhe --verbose move rleft upleft
f virtual_position(u)
f active_move_in_direction(u)
w  vw_target=94457276140256  vw_wall=none  vw_parent=B  vw_groupsize=1
m [con_id=94457275819920] move to mark i34B, focus
m [con_id=94457275819920] focus
f CLEANUP()
[{"success":true},{"success":true},{"success":true}]

this input 'correctly' focuses right

~ > i3Kornhe --verbose move rdown
f virtual_position(r)
f active_move_in_direction(r)
w  vw_target=94457275960096  vw_wall=right-workspace  vw_parent=C  vw_groupsize=1
f family_hide(AC)
m [con_mark=i34XAC] floating enable, resize set 476 1060, move absolute position 1444 px 0 px, move scratchpad
m [con_id=94457275819920] focus
f CLEANUP()
f varset(AC 246 1444)
m [con_id=94457275831344] mark --toggle --add i34FAC=A
m [con_id=94457275831344] mark --add i34FAC=AC
[{"success":true},{"success":true},{"success":true},{"success":true},{"success":true},{"success":true},{"success":true}]

this entry correctly errors out:

~ > i3Korne --verbose move r asdfasd
zsh: command not found: i3Korne
~ > i3Kornhe --verbose move r asdfasd
f virtual_position(a)
[ERROR] 'a' is not a valid argument for --move
f CLEANUP()

Expected behavior

error message / failure on supplying extra arguments.

system

arch btw

~ > i3 --version
i3 version 4.22 (2023-01-02) © 2009 Michael Stapelberg and contributors

**Description** A clear and concise description of what the bug is. - 3) New issue: looking at my i3config, I seem to use i3viswiz to focus; and only seem to use `i3Kornhe move` to actually MOVE my windows across containers. But: it does also exhibit some of the same behaviour. - 3.5 NOTE: i3Kornhe seems to work fine in conjunction with i3fyra: it **does** (after populating my ABCD containers) correctly move/hide/show the ABCD containers as expected from i3fyra (as you can also see in the output below) **To Reproduce** - see below this entry focuses **up**: ```bash ~ > i3Kornhe --verbose move rleft upleft f virtual_position(u) f active_move_in_direction(u) w vw_target=94457276140256 vw_wall=none vw_parent=B vw_groupsize=1 m [con_id=94457275819920] move to mark i34B, focus m [con_id=94457275819920] focus f CLEANUP() [{"success":true},{"success":true},{"success":true}] ``` this input 'correctly' focuses **right** ``` ~ > i3Kornhe --verbose move rdown f virtual_position(r) f active_move_in_direction(r) w vw_target=94457275960096 vw_wall=right-workspace vw_parent=C vw_groupsize=1 f family_hide(AC) m [con_mark=i34XAC] floating enable, resize set 476 1060, move absolute position 1444 px 0 px, move scratchpad m [con_id=94457275819920] focus f CLEANUP() f varset(AC 246 1444) m [con_id=94457275831344] mark --toggle --add i34FAC=A m [con_id=94457275831344] mark --add i34FAC=AC [{"success":true},{"success":true},{"success":true},{"success":true},{"success":true},{"success":true},{"success":true}] ``` this entry correctly errors out: ```bash ~ > i3Korne --verbose move r asdfasd zsh: command not found: i3Korne ~ > i3Kornhe --verbose move r asdfasd f virtual_position(a) [ERROR] 'a' is not a valid argument for --move f CLEANUP() ``` **Expected behavior** - error message / failure on supplying extra arguments. **system** - arch btw ``` ~ > i3 --version i3 version 4.22 (2023-01-02) © 2009 Michael Stapelberg and contributors ```

budRich commented

2023-07-22 12:19:46 +00:00

(Migrated from github.com)

I added some checks and error messages on arguments passed to i3Kornhe. It will not just print errors but also exit on incorrect arguments.

$ i3Kornhe --verbose move rleft upleft
>>> i3Kornhe 
[ERROR] i3Kornhe: mode ('move') expected 0-1 arguments got '(rleft upleft)'

---

i3Kornhe --verbose move rdown
>>> i3Kornhe 
[ERROR] i3Kornhe: direction ('rdown') is not valid! ( left|right|up|down|l|r|u|d|0-9 )

---

i3Kornhe --verbose groove down
>>> i3Kornhe 
[ERROR] i3Kornhe: mode ('groove') is not valid!

But this approach has drawbacks, the additional checks (most are regex..) adds ~5ms to each command, and keep in mind i3Kornhe is the most CPU hogging command of all i3ass, since it very well can be triggered at ~20hz if the user moves/resizes with a keybinding and holds down the keys to repeat the commands.

I didn't experience the lag on my laptop, but it could be noticable on weaker machines or in environments where the CPU load is already high.

The PR (#196) will also break working but miss-spelled configurations. To be honest I haven't decided if i will merge it due to these drawbacks..

Possible alternative solutions would be to only perform these checks if the --verbose flag is set.

In retrospect i regret going with this command argument approach instead of using f.i. --move left . And that is another solution to add support for safer options (--move , --resize). I think that could be added without performance penalties .

I added some checks and error messages on arguments passed to i3Kornhe. It will not just print errors but also exit on incorrect arguments. ``` $ i3Kornhe --verbose move rleft upleft >>> i3Kornhe [ERROR] i3Kornhe: mode ('move') expected 0-1 arguments got '(rleft upleft)' --- i3Kornhe --verbose move rdown >>> i3Kornhe [ERROR] i3Kornhe: direction ('rdown') is not valid! ( left|right|up|down|l|r|u|d|0-9 ) --- i3Kornhe --verbose groove down >>> i3Kornhe [ERROR] i3Kornhe: mode ('groove') is not valid! ``` But this approach has **drawbacks**, the additional checks (most are regex..) adds ~5ms to each command, and keep in mind i3Kornhe is the most CPU hogging command of all i3ass, since it very well can be triggered at ~20hz if the user moves/resizes with a keybinding and holds down the keys to repeat the commands. I didn't experience the lag on my laptop, but it could be noticable on weaker machines or in environments where the CPU load is already high. --- The PR (#196) will also break working but miss-spelled configurations. To be honest I haven't decided if i will merge it due to these drawbacks.. Possible alternative solutions would be to only perform these checks if the `--verbose` flag is set. In retrospect i regret going with this *command argument* approach instead of using f.i. `--move left` . And that is another solution to add support for *safer* options (`--move` , `--resize`). I think that could be added without performance penalties .