Easier to localize? KeyMap.txt refactor - discussion

[sylikc]

@LaurentGrenet I just made a pretty big change to KeyMap.txt which I'm hoping will help localization. Do you think this will help you with localization?

This is how it was previously: KeyMap.txt before the change which combined both the help, the definitions, and the configuration

This is what the new way looks like:

• KeyMap.txt with only the configuration of the KeyMap ... currently there's some text in there, but if I moved all that to the README, then this file should not require any localization
• KeyMap-README.html is just documentation of the KeyMap. I started with a TXT file, but made it HTML because, there's more graphical freedom... but I tried to format the code in a way that makes it easier to localize.
• symbols.km just the definitions with the numbers. This doesn't have to be touched by any user, so it doesn't require localization

I'd like to ask for your feedback to this change, since you have done French localization.

(In the next phase of this refactoring, I'm going to generate some strings.txt and coming up with an automated way to keep that up to date.)

[sylikc]

Oh yeah, don't translate the README.html yet... I may still further improve it based on our discussion here.

[mdnava]

I'd vote to keep it as it currently is. The KeyMap containing the the defs and the keymaps.
If it ain't broken, why fix it?. It's just useful the way it is without having to open other files.
And there are less files, less clutter, less chance of a mess.

A single file is also easier to maintain.. and update whenever new keymaps are added.
In my case for example, I have SHIFT+F2 mapped in JPEGView.ini to open KeyMap.txt for quick editing.
By adding two more files it would require me to open and edit three files, instead of one.

I would recommend however, to have a help page mirrored within the documentation with all available keyboard shortcuts (or at the very least a good number of examples) and how to change them. It is what documentation is for.

Have a nice day!

[sylikc]

I'd vote to keep it as it currently is. The KeyMap containing the help, the defs and the keymaps. If it ain't broken, why fix it?. It's just useful the way it is without having to open other files. And there are less files, less clutter, less chance of a mess.

Thanks for the input. The reason it's being "fixed" is that there's a few problems with it at the moment:

1. KeyMap.txt can be easily broken because the definitions really shouldn't be user accessible. They have to be read by JPEGView, but doesn't need to be touched by a user
2. Having the help tied with the definitions is messy. It's difficult to track changes which need to be reflected in translations. As you see right now, there's only an English and Russian translation... this file is a bear to work with right now
3. Having the definitions tied with the actual map means that it's hard to load the KeyMap from other places. For example, when JPEGView is INSTALLED to C:\Program Files, the directory is not writable by a user. After this change, I would load the symbols from the read-only program directory, but will allow loading KeyMap.txt from like a UserProfile directory, or possibly a command line argument. This allows changing the keymap without touching the definitions (read-only in the program files directory).
4. When new commands are added, users would have to manually merge their current customized KeyMap.txt with whatever new definitions. Either that, or the file overwrites their definitions. Either way, it forces the user to do something to stay up to date. When the file is split, the definitions and help get updated, but the user-configured KeyMap.txt does not have to change.

This is meant to be a pre-cursor to the INI overhaul that's up and coming.

[mdnava]

1. KeyMap.txt can be easily broken because the definitions really shouldn't be user accessible. They have to be read by JPEGView, but doesn't need to be touched by a user

In this context it makes sense to remove the definitions from KeyMap.txt since then we would only need to add new keymaps when available. But is important to not ever modify user customized keymaps. If this ends up being the case, I think the keymap descriptions/help should be moved down to where the actual keymaps are being set, to assist the user.

Anyway. Is not a change I would take lightly.
It could break things.

[LaurentGrenet]

Hi,

personnally, I agree with this split of Keymap.txt in two files.
One (the list of definitions) has never to be updated by the user, while the second one is under user responsibility.
Of course, at first deployment of JPEGView on a Pc, a Keymap.txt has to be generated, with default values. But afterwards, it shoud never be overwritten by any new version deployment, whatever this deployment is the result of an "Install", or only unzipping of distrib ("portable")

Even if list of options has never to be updated by any user, it's nevertheless useful to have it in a text file, in order to allow users to check the name (and exact spelling) of various options.

On the other hand, I agree with @mdnava when he writes "less files, less clutter, less chance of a mess"
So my recommendation would be to completely REMOVE the keymap.txt file, and rather include keymaps in the Jpegview.ini file.

And I repeat (and detail a little bit more) what could be the best (IMHO) process to manage this .ini file (or THESE files in case keymap.txt remain distinct from Jpegview.ini) in order to

• provide default values to new users,
• and never overwrite options previously customized by a user when deploying a new version :

Add a new entry in Jpegview.ini storing the Jpegview version that last updated it (eg. let's name it "JpegViewVersion")
At each launch of the app
	Is there a personal .ini file ?
	If yes
	Then
		Read it
		if "JpegViewVersion" <> current version
		Then
			Add new entries provided with the new version of app (initialized with their default value)
			Update the "JpegViewVersion" parameter with current value
		End If
		Use it as config for current session
	Else 
		Is there a global .ini file ?
		If Yes
		Then
			Read it
			if "JpegViewVersion" <> current version
			Then
				Add new entries provided with the new version of app (initialized with their default value)
				Update the "JpegViewVersion" parameter with current value
			End If
			Use it as config for current session
		Else
			Create a global Jpegview.ini with all default values
			Use it as config for current session
		End If
	End If

[sylikc]

@LaurentGrenet Thanks for your input. Unfortunately the KeyMap.txt and JPEGView.ini can't be merged. I thought quite long and hard about this one, but the semantics of KeyMap is different than INI.

INI is options. It is based on one value being something. KeyMap is very flexible and permits anything to be anything. In INI can be "loaded in a hierarchy", aka, system options + user options, but that doesn't make sense for a KeyMap.

For example, if a global INI had

A=true
B=1
C=Test

and a user INI had

A=false
C=Hello

You can read the values of A, B, C and it works as expected.

In a keymap, in the same example

A    COMMAND_A
B    COMMAND_B
C    COMMAND_C

user Keymap

A    COMMAND_B
C    COMMAND_A

what happens when you press B? etc etc. The semantics of the file and what it's trying to do is inherently different...

(Also, while the whole "merging" INIs flow up above sounds nice, it also doesn't quite work like that in code. We can discuss that on a separate thread though. I'm not ready to tackle the INI problems yet.)

[LaurentGrenet]

Whatever files are merged or not, I don't think it's a good thing that INI file is "loaded in a hierarchy" as you say.
I cannot imagine the need, but I can very easily imagine the complexity it can generate.

For me, It would be a lot easier to understand if we didn't have this hierarchy. If there is a "user" INI file, the global file should be completely ignored. And the global INI file taken into consideration ONLY if there is no User INI.
This is actually consistent with the fact that the admin/settings menu "Update user settings" create in the user ini ALL the entries. Not only the one modified.

In the example you ask for above (with keymap) for me, without any doubt nor hesitation, nothing should happen when I press B, since there is a user Keymap and this user Keymap do not mention key B.

Currently, the concept of "user keymap" do not exist. If you do not implement it, and if you do not implement a flow like the one I describe to manage it when new version of JPEGView comes, I really do not see the interest to split existing file in two....
In such a case, let everything as it is currently.

For me, the ONLY real issue we are facing today is how to preserve user settings when a new version comes...
Personally I manage it manually, keeping a copy of "my" files before deploying the new version, then comparing (manually) them with the new files delivered by the new version, and "merging" both (always manually).
This process and only this one, according to me, has to be enhanced.

[sylikc]

Ok ok. It's fine... lol. The restructure is EXACTLY to allow a user keymap. You can't have a user keymap when the definitions are tied to the keymap. You'll see.

So, your specific use case is managing stuff manually because you're not installing. But I also have to consider the installed use case... this whole problem isn't to solve one use case. It needs to work with both, and there is actually other use cases, specifically like PortableApps, or other installers. But it's fine. Like I said, this discussion wasn't for that issue.

Let me go back to the original question. Is it easy to localize an HTML file the way it is. I'm going to put the strings out into one area and it will render with CSS.

[mdnava]

Ok ok. It's fine... lol. The restructure is EXACTLY to allow a user keymap. You can't have a user keymap when the definitions are tied to the keymap. You'll see.

That makes sense. I suppose it is the right reason to separate the definitions from the keymaps. Then I believe JPEGView should just start ignoring the definitions in any KeyMap.txt files current users have in place. So any definitions in KeyMap.txt would have no effect whatsoever. This shouldn't have any consequence to the user.

@sylikc if you're reviewing the JPEGView.ini file please consider adding up an option for items in "User commands" and/or "Open with" to be hidden from the context menus. In a way that we could have some commands or utilities available with keyboard shortcuts without cluttering those menus. I already have both menus pretty filled up.

[LaurentGrenet]

Ok, let's go back to the original question.

And then I ask my own question : Are you sure it worth localizing such a file ? Personnally, I'm not convinced (at all !).
This HTML file is more or less a "Help" file for advanced users, and IMHO this kind of file may remain only in English.
It is not dedicated to all users, but only to advanced users, skilled enough to update the file.... It is not the same level of difficulty than modify options in a menu, and at the end clic a menu "Set current parameters as default value"
According to me, users that are able to do so, (the same than those that are able to choose the "Edit users settings" to update it in Notepad or equivalent) should be able to deal with a "how to" documentation only in english

Nevertheless, if you prefer to localize it, and we have a similar way of doing than for other localization (ie. a list of pairs english text / localized text) I don't understand how, when and where will be generated the localized HTML ? This is quite different from localizing GUI of the application...
The only way I can see would be to directly rewrite the full HTML file in different locales, and have a Keymap-README-xx.txt (where xx is locale's code)
But I repeat, I don't see the real "pay off" of such a localization.

[sdneon]

• Don't split it into too many scattered pieces, making them difficult to find for future developers

• #defines are unnecessary - there're more like abbreviated descriptions, so trash them, as there are comments serving the same purpose.

• New KeyMap just need to be brief like this:

//Look in KeyMap-README.HTML for detailed list of all commands available
//<numeric id as per internals> [optional  hotkey] [//optional user comment/description]
//e.g.
1010 //minimize window; no hotkey
1011 W // open and select file in Explorer
1500 Ctrl+S // save to file in original size, don't allow override without prompt
1600 Ctrl+R // reload image
6000 F7
6010 F8
6020 F9

• list all possible command IDs from resource.h

  • Currently, there are unlisted commands in KeyMap.txt, and they could have needed hotkeys. Only a developer digging deep into the codes will find them; so list them for common folks to know.

• comments could either:

  • always be provided; as description of each command ID
  • or just for user to doodle their thinking when tweaking the hotkeys

• allow no hotkey specified.

  • The ID entry serves as info that there is such a command ID (in this case, probably should have a brief description for popular commands likely to need hotkey). Interested users can then assign a hotkey if desired, and lookup README if needed.

• with separate, translatable table of IDs to description (& default hotkey), in the proposed KeyMap-README.HTML

• It is essential to allow users to customize hotkeys. Especially to remove the many unwanted hotkeys, like things which cause accident edits and saves.

Or best: have a runtime dialog box to configure hotkeys, with live capture of hotkey.

[sylikc]

Thanks for the inputs @sdneon .

• #defines are unnecessary - there're more like abbreviated descriptions, so trash them, as there are comments serving the same purpose.

While the defines are (in theory) unnecessary, I think it makes the KeyMap a lot more readable by mapping the numeric IDs to brief descriptions. Just like DNS entries are unnecessary if everyone could just use IP addresses.

• New KeyMap just need to be brief like this

I do want to flip the order though like your example, where the ID is on the left, and the keys are on the right. That might come in the future. It would require existing users to do the inversion though, and break everyone's keymaps.

• list all possible command IDs from resource.h

As for the IDs from resource.h, I did notice a lot of them not being exposed, but that's also probably to keep the list manageable... I'll think about adding them all. Probably just need a short blurb comment to each of them so it propagates to the keymap defines.

Or best: have a runtime dialog box to configure hotkeys, with live capture of hotkey

If you want to code this up and issue a PR, I'll take it.

One of the reasons I did this refactor in this specific way is that it doesn't affect existing KeyMaps. The code changes minimally for the features that are gained. The change is actually invisible to users, as their existing #defines will be ignored, and the KeyMap.txt file won't be overwritten (I'm renaming the default in a few commits)