Keymap UK_en cannot be set with loadkeys, and GB_en does not exist.

Patch attached to fix this.

Simply a class rename from org.jnode.driver.input.l10n.KeyboardInterpreter_UK_en
to the correct name
org.jnode.driver.input.l10n.KeyboardInterpreter_GB_en

Works for me.

Hi Brett,
Your patch has been committed.
Many thanks,
Levente

Levente pointed out, that there is still a remaining problem:

"Further, the auto completion/suggestion on the loadkeys command shows all the country and language codes irrespective of whether a KeyboardInterpreter exists (this is probably a separate bug, I just wanted to mention it)."

So reopening this issue, sorry. I think Brett even mentioned to work on a fix for that.

Yep, I'm looking at this.

I'd like us to have standard pattern for specialising the behaviour of Arguments and completion. Here are some examples of behaviour that we might want to support.

• This one ... accept only ISO language codes that correspond to supported keyboard maps.
• Devices that implement a specific device API. (The DeviceArgment class currently implements this in an ad-hoc way.)
• Files that exist, do not exist, are files, are directories, etc.
• Files with specified suffixes.
• Subranges of integers, etc. (Though maybe this is stretching the abstraction a bit too much ...)

I can think of one way to support many of these examples. In broad terms, it could be done with a wrapper class that implements the Argument API. The wrapper would filter the tokens accepted and completed by the wrapped Argument class according to the required behaviour.

Hm, I thought the argument api was a pretty good abstraction anyway.

I've only looked at the example of loadkeys, but needed to specialise beyond the CountryArgument and LanguageArgument to arguments that can speak to an object that knows which KeyboardInterpreters are installed.

There is mention in the docs that loadkeys should be able to support arbitrary keymaps (for broken keyboards or obscure personal setups) that will be non-standard. The variant string seems to fit these.

I don't know what you mean by a wrapper that implements the Argument API, atm commands specify by class the Argument implementing classes that they expect as arguments. Those arguments are responsible for providing completion and validation. I'm sure I'm missing a lot here, given that I'm digging specifically rather than generically.

I can't speak to the other examples, I need to look further and hopefully chat with you.

The attached class is a first cut at what I'm suggesting. (It needs to be tested ...)

You know ... the "loadkeys" example is going to be rather tricky.

For a start, keyboard layouts are identified 1, 2 or 3 part tuples, depending on the layout. To complete (say) the language code argument against the available layouts, the LanguageArgument needs to know what the CountryArgument in the same bundle is bound to. This goes against the current conceptual model where each Argument instance in a bundle is independent of the others in the bundle, and the Syntax is free to bind them using a variety of syntactic constructs ... in any order.

To make it worse, the KeyboardInterpreterFactory API does not provide a method for iterating over the supported layouts. I'm not even sure it is implementable without adding some kind of layout table to the relevant plugin descriptor ... or something like that.

(Aside: many of the supported layouts are the same in all but name; e.g. "HU", "HU_hu", "HU_101" and "HU_hu_101". I cannot help thinking that keyboard layouts would be better handled by reading them from data files rather than embedding them in Java classes.)

Well I've got a working loadkeys example here that I'm just cleaning ready to give you a patch for.

To get the list of installed interpreters I used the classloader(PluginLoader) of the default interpreter after it's loaded by the factory. I'm sure there is a better way, but my JNode knowledge is too shallow.

The patch doesn't change any API's so at best it would be a stop-gap until they are updated. I'll clean it and submit it anyway, even though I suspect it'll be rejected

"(Aside: many of the supported layouts are the same in all but name; e.g. "HU", "HU_hu", "HU_101" and "HU_hu_101". I cannot help thinking that keyboard layouts would be better handled by reading them from data files rather than embedding them in Java classes.)"

Yeah, I really wanted to change the argument to loadkeys to be a single argument. Would have been a lot easier.

I didn't want to have the interpreter names anywhere other than the class files if it's not necessary. Just makes it harder to add a new one. The loadkeys doc says it would be nice to have user-defined interpreters, doing this on-the-fly while running is easy if you only have to build a class to the filesystem and register it's name.

Anyway, here's my patch, just for giggles.
Works for me.

Argument.vArray is currently private, np.

Can you tell me what this offers that the current Argument doesn't already? I got the impression from the existing Arguments that complete(...) was a reasonable place to do selective completion already? Is this an API cleanliness issue?

complete(..):
* Subtypes of Argument should override this method if they are capable of doing
* non-trivial completion. Completions should be registered by calling one
* of the 'addCompletion' methods on the CompletionInfo.

I'm not convinced one way or the other about my proposed "general" solution versus simply creating subclasses of existing Argument classes. In simple cases, it is easy to just implement the new behavior in a subclass. In cases where the existing class has a complex "complete" method, you have a bit of a problem. Either you copy the code in the subclass, or you call "super.complete(tmpCompletion, partial)" and jump though hoops to reprocess the completions. (Or you hack the original class to support the new use-case.)

I think we should not handle every special cornercase in the generic command framework. Instead it should be possible to handle the special cases in place where we implement the command. The framework should be sufficiently extensible and flexible to support this.

The loadkeys command is certainly such a case and the current solution as we define and load keymaps doesn't seem to be satisfactory. The related stuff in the sourcecode is old and it needs to be revisited.

Beyond the bare focus on how to implement code completion for loadkeys command we need a better way to manage keymaps. We need a keymap manager which can be easily used for implementing the command completion but also for building other tools for keymap configuration. Now our Swing based GUI is getting to a point where we can start thinking about a graphical configuration tool as well for various features of JNode, including the keymaps. A keymap manager should be a useful and reusable component in this context too.

On a high level the keymap manager should be able to load a selected keymap without touching all the other keymaps. Only a request for discovering the keymaps should scan all available keymaps. The reason for this is that in perespective there will be many keymaps and loading them all on startup is too expensive both performance and memorywise and completely unnecessary. The discovery mechanism of keymaps should be fast since it will be visible for the user on command completion or keymap configurator gui startup.

Regarding the keymap identifiers, local names, two letter ISO country and languages codes can be useful in constructing them but they are not as closely related as they seem. For instance: how do you handle in a completely locale based approach the Dvorak keymaps? Keymap variants mostly escape the possibilities of java.util.Locale too. So we need to define the keymap identifier format and use the identifiers in loading keymaps as well as in completing the loadkey command arguments. Identifiers will solve the problem surfacing in the case of the hungarian keyboard interpreter where in the lack of identifiers all the possible ways the user might want to load the keymap are encoded in classnames which represent functionally the same class. A very uggly and desperate hack of the distant past indeed and but the same would happen with other keymaps too. Obviously for one keymap there shold be one class (or other entity which represents it) and the keymap should declare its identifier, maybe specify the locale too to which it can be related. The keymap manager should handle the mapping between locales and keymap indetifiers (for suggesting a default keymap for a locale, as an example) if such mapping will be needed.

So this was only an informal attempt to set the scene for this problem.
Ideas, suggests are most welcome.

Comments on the patches:

1. The patches to "KeyboardInterpreterFactory.java" have lots of style issues. Next time, please run "build.sh checkstyle" before you create a patch.
2. The custom Argument types for "loadkeys" make the assumption that they will be bound in the Syntax in a specific order. If the someone (e.g. the end user) decides to redefine the command syntax to put (say) the language in front of the country, then completion won't work.
3. I think that the way to address the previous issue is to redo the factory API so that the caller supplies a simple string which is the composite identifier for the layout, and the user supplies this identifier as a single command-line argument.

1. Ah ok, will do.
2. Oh, I didn't even know that was possible.
3. A single argument sounds better.

Thanks for giving them a look

I am working on changes to keyboard layout management that will (when completed) allow listing of all available keyboard layouts, allow registration of new layout classes on the fly, and allow alternative layout names to be used w/o the overheads of the current approach.

Sounds very interesting Stephen. I promised Sergey a Russian/Latin keyboard handling, which I planned to start looking at soon, maybe we should talk about how to handle things like this together over our irc or by mail.

The 'loadkeys' command now supports entry of layout ids as a single argument (rather than a triple) and offers completion against the set of registered layouts. The command now has options to '--set' the layout for the keyboard(s), '--add' a new keyboard layout -> interpreter mapping and '--remove' one.

Run 'help loadkeys' for the revised command syntax.

I think I've covered everything raised by this issue, so I'm marking it fixed.

Manually closing.