Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Per-layout command / more flexible commands #15

Open
michaelcadilhac opened this issue Mar 1, 2023 · 3 comments
Open

Per-layout command / more flexible commands #15

michaelcadilhac opened this issue Mar 1, 2023 · 3 comments

Comments

@michaelcadilhac
Copy link

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
Copy link
Owner

echuraev commented Mar 3, 2023

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: #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
Copy link
Author

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:

  • The user is happy with having the cmd/sub_cmd divide; it saves them a bit of typing, and makes the config file ever so slightly neater. (As a personal note, I'm not a fan of this artificial divide; if I, as a user, want to factor the cmd part, then I could just define that variable, and use cmd .. " dvorak" in a "full command" interface. I'm more likely to understand my own config file that way.)
  • The user wants flexibility. In that case, they can enter the full command if they so wish. This accommodates most flexibility needs — arguably, the user may even want to provide a Lua function for the full command.

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
Copy link
Owner

echuraev commented Mar 3, 2023

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:

  • It was described in my comment above. We can add an argument to factory method and specify if we want to use quotes or not. In this case, we still will use separation on cmd and sub_cmd but it should make commands a bit more workable for your case.
  • Introduce an argument to factory method which will specify if user wants to use full_cmd instead of sub_cmd. In this case, It is assumed that all commands which were passed to sub_cmd will be interpreted ad a full_cmd and cmd will be ignored.
  • This follows from the previous item. Add an optional argument to add_primary_layout which will specify if sub_cmd should be used as a full_cmd. And the command can be like this: kbdcfg.add_primary_layout("Dvorak", "DV", "setxkbmap -option compase:ralt dvorak && xmodmap ~/.xmodmaprc", true).
  • Add an optional argument to add_primary_layout which allows user to determine full_cmd. It's your proposal, and it also looks good to me, but for now I don't imagine the good API for such function. I have to refresh my Lua knowledge and remember its syntax. Probably you could share your vision how this function should be called from the user configs.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants