echuraev / keyboard_layout

Keyboard switcher for Awesome WM with additional layouts
MIT License
74 stars 13 forks source link

Per-layout command / more flexible commands #15

Open michaelcadilhac opened 1 year ago

michaelcadilhac commented 1 year ago

My use case is that I alternate between US and US Dvorak. For both, I have a setxkbmap option and I then reload ~/.xmodmaprc. I use this rather unfortunate line:

kbdcfg.add_primary_layout("Dvorak", "DV", "-option\" compose:ralt  dvorak && xmodmap ~/.xmodmaprc && echo \"")

This is to "hack" into the os.execute that is done by the code.

A much more elegant solution would be to have a add_primary_layout with named arguments name, icon, full_command, cmd_argument, with the last two optional, although one of the two should be there. For instance:

kbdcfg.add_primary_layout_opt { name = "Dvorak", icon = "DV",
   full_command = "setxkbmap -option compase:ralt dvorak && xmodmap ~/.xmodmaprc" }

xbdcfg.add_primary_layout_opt { name = "US", icon = "US", cmd_argument = "us" }

Alternatively, we could specify the command when creating kbdcfg as:

cmd = "setxkbmap -option compose:ralt \"{}\" && xmodap ~/.xmodmaprc"

… but this does not provide the same flexibility.

echuraev commented 1 year ago

Hi @michaelcadilhac!

Thank you for your feedback. Just another option how you can solve your issue. As far as I understand, the issue is in quotes surround sub_cmd which was introduced in this PR: https://github.com/echuraev/keyboard_layout/pull/9

You can introduce an argument to the factory method which will specify if sub_cmd should be surrounded by quotes or not. By default, it should be surrounded. In this case, when the user specifies that quotes should not be used, he/she should explicitly specify quotes where it is necessary, e.g.: kbdcfg.add_primary_layout("Czech (QWERTY)", "CZ", "\"cz(qwerty)\"").

In this case, no overloading will be introduced and it will work without modification of config files for all current users. I'm not sure that adding overloading won't require config files modification.

michaelcadilhac commented 1 year ago

Hello and thanks for the prompt reply!

I believe that if we do want to make the interface more flexible, keeping the divide cmd/sub_cmd is just getting in the way. For the end user, I see two extremes:

I understand you are preoccupied by backward compatibility; we can just add an optional argument for add_primary_layout that takes the full command. That'd solve pretty much everything. What do you think?

echuraev commented 1 year ago

The initial idea why sub_cmd was added is to relieve the user of the need of defining the command which changes keyboard layout, and accordingly it is not necessary for the user to dive into implementation details. Also, cmd helps not to duplicate part of the code for each new keyboard layout, you only need pass in sub_cmd the layout which should be used. For the advance users or some not common cases, cmd option was introduced.

What about flexibility. I agree it is a very important part of this widget and I definitely agree with your arguments, but I don't want to break backward compatibility and force users to write full command. I think it might be inconvenient. If we speak about adding optional argument to add_primary_layout then one of the argument should be required: sub_cmd or full_cmd. I believe it should be sub_cmd because of backward compatibility. But how we should call add_primary_layout in this case? Like this: kbdcfg.add_primary_layout("Dvorak", "DV", "", "setxkbmap -option compase:ralt dvorak && xmodmap ~/.xmodmaprc")? Sorry, I'm not an expert in Lua and last time a wrote something in Lua a couple of years ago and now (probably till the end of this month) I don't have access to Linux machine and I cannot check how it will work and implement some prototype.

I see several possible options:

Thank you for this good feedback. I'll be glad to see such flexibility improvements in the widget.