[Bf-blender-cvs] [945ea40] master: Fix T41757: Inconsistent hotkey label with setting.
Bastien Montagne
noreply at git.blender.org
Thu Jan 29 17:40:16 CET 2015
Commit: 945ea40887bafff14ad40644b8a86af28bc1ae5c
Author: Bastien Montagne
Date: Thu Jan 29 17:32:34 2015 +0100
Branches: master
https://developer.blender.org/rB945ea40887bafff14ad40644b8a86af28bc1ae5c
Fix T41757: Inconsistent hotkey label with setting.
Issue is double here:
* Quite a handfull of menu entries actually diverge slightly from their shortcut
counterpart (often one has a prop explicitely set to its default value,
when the other keep it unset).
* Current code was actually basically sending 'is_strict' option into canal,
by doing a second check in `wm_keymap_item_find` setting unset op props
to their default value!
Now, is_strict mostly says one thing: "never consider an unset property as
equal to a set one". Even if set property matches default value. Default values
are not always the same things as unset ones, as demonstrated by this report.
So we are being much stricter now, and also have to check shortcuts and
menu entries definitions actually matches, added some code (triggered by
--debug-wm option) that prints when it finds some (potential) issue.
There is one exception though - Macros. Those have their whole prop set defined
in menu entries currently, this shall probably not be the case, but is another issue,
so for now for macro operators we always do non-strict comparison (pretty much
the same as previously, in this case).
Also 'enum' operators are still tricky. Currently, shortcut extraction relies on
`ot->prop` being set, so even if this is not aboslutely needed anymore (when defining
UI you can specify an arbitrary enum property by name), `ot->prop` shall be set.
Note fix commit for mismatches between menu entries and shortcuts is needed next.
===================================================================
M source/blender/windowmanager/intern/wm_keymap.c
===================================================================
diff --git a/source/blender/windowmanager/intern/wm_keymap.c b/source/blender/windowmanager/intern/wm_keymap.c
index a164304..a79b992 100644
--- a/source/blender/windowmanager/intern/wm_keymap.c
+++ b/source/blender/windowmanager/intern/wm_keymap.c
@@ -953,7 +953,6 @@ static wmKeyMapItem *wm_keymap_item_find_handlers(
}
if (properties) {
-
/* example of debugging keymaps */
#if 0
if (kmi->ptr) {
@@ -970,6 +969,39 @@ static wmKeyMapItem *wm_keymap_item_find_handlers(
if (keymap_r) *keymap_r = keymap;
return kmi;
}
+ /* Debug only, helps spotting mismatches between menu entries and shortcuts! */
+ else if (G.debug & G_DEBUG_WM) {
+ if (is_strict && kmi->ptr) {
+ wmOperatorType *ot = WM_operatortype_find(opname, true);
+ if (ot) {
+ /* make a copy of the properties and set unset ones to their default values. */
+ PointerRNA opptr;
+ IDProperty *properties_default = IDP_CopyProperty(kmi->ptr->data);
+
+ RNA_pointer_create(NULL, ot->srna, properties_default, &opptr);
+ WM_operator_properties_default(&opptr, true);
+
+ if (IDP_EqualsProperties_ex(properties, properties_default, is_strict)) {
+ char kmi_str[128];
+ WM_keymap_item_to_string(kmi, kmi_str, sizeof(kmi_str));
+ /* Note gievn properties could come from other things than menu entry... */
+ printf("%s: Some set values in menu entry match default op values, "
+ "this might not be desired!\n", opname);
+ printf("\tkm: '%s', kmi: '%s'\n", keymap->idname, kmi_str);
+#ifndef NDEBUG
+ printf("OPERATOR\n");
+ IDP_spit(properties);
+ printf("KEYMAP\n");
+ IDP_spit(kmi->ptr->data);
+#endif
+ printf("\n");
+ }
+
+ IDP_FreeProperty(properties_default);
+ MEM_freeN(properties_default);
+ }
+ }
+ }
}
else {
if (keymap_r) *keymap_r = keymap;
@@ -1036,33 +1068,83 @@ static wmKeyMapItem *wm_keymap_item_find_props(
static wmKeyMapItem *wm_keymap_item_find(
const bContext *C, const char *opname, int opcontext,
- IDProperty *properties, const bool is_hotkey, const bool is_strict, wmKeyMap **keymap_r)
+ IDProperty *properties, const bool is_hotkey, bool is_strict, wmKeyMap **keymap_r)
{
- wmKeyMapItem *found = wm_keymap_item_find_props(C, opname, opcontext, properties, is_strict, is_hotkey, keymap_r);
+ wmKeyMapItem *found;
+ /* XXX Hack! Macro operators in menu entrie have their whole props defined, which is not the case for
+ * relevant keymap entries. Could be good to check and harmonize this, but for now allways
+ * compare non-strict in this case.
+ */
+ wmOperatorType *ot = WM_operatortype_find(opname, true);
+ if (ot) {
+ is_strict = is_strict && ((ot->flag & OPTYPE_MACRO) == 0);
+ }
+
+ found = wm_keymap_item_find_props(C, opname, opcontext, properties, is_strict, is_hotkey, keymap_r);
+
+ /* This block is *only* useful in one case: when op uses an enum menu in its prop member
+ * (then, we want to rerun a comparison with that 'prop' unset). Note this remains brittle,
+ * since now any enum prop may be used in UI (specified by name), ot->prop is not so much used...
+ * Otherwise:
+ * * If non-strict, unset properties always match set ones in IDP_EqualsProperties_ex.
+ * * If strict, unset properties never match set ones in IDP_EqualsProperties_ex,
+ * and we do not want that to change (else we get things like T41757)!
+ * ...so in either case, re-running a comparison with unset props set to default is useless.
+ */
if (!found && properties) {
- wmOperatorType *ot = WM_operatortype_find(opname, true);
- if (ot) {
- /* make a copy of the properties and set any unset props
- * to their default values, so the ID property compare function succeeds */
+ if (ot && ot->prop) { /* XXX Shall we also check ot->prop is actually an enum? */
+ /* make a copy of the properties and unset the 'ot->prop' one if set. */
PointerRNA opptr;
- IDProperty *properties_default = IDP_CopyProperty(properties);
+ IDProperty *properties_temp = IDP_CopyProperty(properties);
- RNA_pointer_create(NULL, ot->srna, properties_default, &opptr);
+ RNA_pointer_create(NULL, ot->srna, properties_temp, &opptr);
- if (WM_operator_properties_default(&opptr, true) ||
- (!is_strict && ot->prop && RNA_property_is_set(&opptr, ot->prop)))
- {
- /* for operator that has enum menu, unset it so it always matches */
- if (!is_strict && ot->prop) {
- RNA_property_unset(&opptr, ot->prop);
- }
+ if (RNA_property_is_set(&opptr, ot->prop)) {
+ /* for operator that has enum menu, unset it so its value does not affect comparison result */
+ RNA_property_unset(&opptr, ot->prop);
- found = wm_keymap_item_find_props(C, opname, opcontext, properties_default, false, is_hotkey, keymap_r);
+ found = wm_keymap_item_find_props(C, opname, opcontext, properties_temp,
+ is_strict, is_hotkey, keymap_r);
}
- IDP_FreeProperty(properties_default);
- MEM_freeN(properties_default);
+ IDP_FreeProperty(properties_temp);
+ MEM_freeN(properties_temp);
+ }
+ }
+
+ /* Debug only, helps spotting mismatches between menu entries and shortcuts! */
+ if (G.debug & G_DEBUG_WM) {
+ if (!found && is_strict && properties) {
+ wmKeyMap *km;
+ wmKeyMapItem *kmi;
+ if (ot) {
+ /* make a copy of the properties and set unset ones to their default values. */
+ PointerRNA opptr;
+ IDProperty *properties_default = IDP_CopyProperty(properties);
+
+ RNA_pointer_create(NULL, ot->srna, properties_default, &opptr);
+ WM_operator_properties_default(&opptr, true);
+
+ kmi = wm_keymap_item_find_props(C, opname, opcontext, properties_default, is_strict, is_hotkey, &km);
+ if (kmi) {
+ char kmi_str[128];
+ WM_keymap_item_to_string(kmi, kmi_str, sizeof(kmi_str));
+ printf("%s: Some set values in keymap entry match default op values, "
+ "this might not be desired!\n", opname);
+ printf("\tkm: '%s', kmi: '%s'\n", km->idname, kmi_str);
+#ifndef NDEBUG
+ printf("OPERATOR\n");
+ IDP_spit(properties);
+ printf("KEYMAP\n");
+ IDP_spit(kmi->ptr->data);
+#endif
+ printf("\n");
+ }
+
+ IDP_FreeProperty(properties_default);
+ MEM_freeN(properties_default);
+ }
}
}
More information about the Bf-blender-cvs
mailing list