GNOME Bugzilla – Bug 758051
Support free-form shortcuts in GtkShortcutsWindow
Last modified: 2015-11-15 02:41:04 UTC
Created attachment 315396 [details] [review] shortcuts: Support free-form shortcuts Some applications define shortcuts that gtk_accelerator_parse doesn't understand, and which are defined in another way inside the application. The attached patch adds a free-form property to GtkShortcutsShortcut and GtkShortcutLabel. I wasn't sure how far to go since I'm not sure if the general direction is correct (i.e. are get_free_form/set_free_form in GtkShortcutLabel even necessary?). This version also doesn't support multiple free-form shortcuts in one GtkShortcutLabel. You also can't mix them, maybe it would be better to just fall back to free-form shortcuts when gtk_accelerator_parse can't parse them? OTOH, if you want valid accelerators, the warning is helpful...
I like having an explicit marker property like this. It would be good to support the same variation we do with standard accels: alternative ::= ((range|accel) space)+ range ::= accel dots accel where an individual accel will get formatted as [key1] + [key2] + ... a range will get formatted as accel ... accel an alternative will get formatted as accel / accel and several keys have Unicode replacements I think one more formatting variant we likely want to allow if we go freeform is [key1][key2] to mean: press key1 and then key2 (as opposed to [key1]+[key2] which means: press key1 and key2 together).
(In reply to Matthias Clasen from comment #1) > I like having an explicit marker property like this. > > It would be good to support the same variation we do with standard accels: > > alternative ::= ((range|accel) space)+ > range ::= accel dots accel > > where an individual accel will get formatted as [key1] + [key2] + ... > a range will get formatted as accel ... accel > an alternative will get formatted as accel / accel > and several keys have Unicode replacements My definition of "free-form" was basically "don't transform the given string in any way and just display it", i.e. it just circumvents the current transformations GtkShortcutLabel does. > I think one more formatting variant we likely want to allow if we go > freeform is > [key1][key2] to mean: press key1 and then key2 (as opposed to [key1]+[key2] > which means: press key1 and key2 together). That's really the case I care about here. So you suggest to just do the normal loop in gtk_shortcut_label_rebuild we're currently doing, but not do the gtk_accelerator_parse if we are freeform? Or do it anyway and don't warn if the _parse fails and we are freeform? (that would mean the stuff in get_labels would still work)
I wasn't suggesting anything in particular yet... but if you are mainly interested in [key1][key2], then maybe we just need to extend the syntax of the current parser to allow specifying it, say with a - between accels (in your case, the 'accels' will be plain keys, but nevermind). The nesting gets a little iffy maybe, with all of space, ..., and - possible to appear between accels, and nesting. May need to look at allowing () to disambiguate. input formatted as <Control>u [Control]+[u] <Shift>Insert <Control>c [Shift]+[Insert] / [Control]+[c] a...z [a]...[z] t-t [t][t] <Control><Shift>u-1-2-3 [Control]+[Shift]+[u] [1][2][3] Maybe problematic cases: <Control>- <Control>...
Created attachment 315451 [details] [review] GtkShortcutLabel: Support double-key accels This uses a + instead of a - (made more sense to me). the outcome for t+t is [T][T]. One problem with this patch is that e.g. <ctrl>+ won't work (I'm guessing this makes sense in some locales). We could just check whether the + is at the end of the current accel. Anyway, "++" won't be a possible accel, not that I care.
Created attachment 315452 [details] Screenshot of the outcome
Umm, just realized that patch still has the free_form change in it m(
Luckily, <Control>+ is not a valid accelerator anyway. You are supposed to write <Control>plus
I decided to rewrite the parser to make it more obvious what is going on.