firecat53 / keepmenu

Dmenu/Rofi frontend for Keepass databases
GNU General Public License v3.0
207 stars 32 forks source link

Remove '-L' option from wofi command #146

Open ndcontini opened 1 year ago

ndcontini commented 1 year ago

In menu.py, -L is used to specify the height of the menu. However, this interferes with the user's wofi settings. For example, I have set wofi to spawn on the left side of the screen with the height of the entire screen. The -L overrides the height setting I've made and instead makes wofi spawn at approximately a third of the height of the entire screen. I'm not entirely certain how useful it is to specify this parameter by default; in my mind it should be up to the user to configure wofi how they would like or wrap keepmenu appropriately if they want a keepmenu specific configuration. Right now I have to work around this by editing menu.py, but it'd be nice if the parameter was removed upstream or at least a user options to control this behavior was added.

Note: I'm not sure how this is affects other menus like rofi and what not.

https://github.com/firecat53/keepmenu/blob/14b27829a2a3259fe826c4cead44bc2f8be4684e/keepmenu/menu.py#L20C1-L36C19

To reproduce: 1) edit $XDG_CONFIG_HOME/.config/wofi/config and add the option height=100%, or any user specified height really. 2) run keepmenu. You'll see that this setting is not kept, despite other wofi instantiations working correctly.

firecat53 commented 1 year ago

This one is a bit complicated. Keepmenu attempts to set a dynamic height based on the number of lines of input. There's no provision in keepmenu for any of the launchers to set a completely static height, just a maximum height.

None of the other supported launchers have a --height parameter like Wofi that is set in pixels or percent, just a --lines for a max number of lines to show.

I'll leave this open in case someone wants to attempt a PR.

Hopefully I at least answered your questions!

Edit: I did test Wofi and the -L parameter does seem to override the -H parameter no matter what order they are passed.

ndcontini commented 1 year ago

The thing I don't understand is why it is beneficial to pass this parameter from keepmenu to the underlying launcher. From my point of view it is an anti-feature because it is overriding my launcher's config. Do the dimensions of the launcher seem off if the max number of lines isn't passed or something? Is this essentially to prevent the entries in the launcher from spilling off the screen?

firecat53 commented 1 year ago

I see your concern. To my eyes, it seems that having a large launcher window is "ugly" when I'm only displaying a few entries or requesting a password. If I'm understanding you correctly, you like a fixed-sized launcher window no matter how many entries there are?

It would be fairly easy to take away the -l option and just let the launcher parameters decide how big the window is. However, it's not so easy to allow a 'dynamic' size from one line for a password up to a max height that is defined by -l while still allowing for -l to be the fixed height if desired. Not impossible...just more complex.

treeshateorcs commented 2 months ago

i use wmenu and i agree with @ndcontini, the hardcoded -l parameter (here https://github.com/firecat53/keepmenu/blob/6d41ce956a11259df935b34f82dc5b0c5e7fd2ea/keepmenu/menu.py#L22 ) is not necessary. i just don't like the look of a vertical menu, and have to remove it on every install. i propose to remove it from main.py and instead add it to the default config

p.s. not to mention that it's already there

https://github.com/firecat53/keepmenu/blob/6d41ce956a11259df935b34f82dc5b0c5e7fd2ea/config.ini.example#L6