Fix T79640: "Assign Shortcut" doesn't work for "View 2D Zoom"
ClosedPublic
Actions

Authored by Philipp Oeser (lichtwerk) on Aug 10 2020, 12:33 PM.

Details

Reviewers

Julian Eisel (Severin)
Campbell Barton (campbellbarton)

Maniphest Tasks

T79640: "Assign Shortcut" doesn't work for "View 2D Zoom"

Summary

'VIEW2D_OT' operators were not respected in WM_keymap_guess_opname().
This was seemingly done on purpose (see comment "Op types purposely
skipped for now"), but dont really see the reason for doing so.

Since the "View2D" keymap is not bound to a specific spacetype, we can
still find it using WM_keymap_find_all() [and passing 0 as spacetype] or
better use the dedicated WM_keymap_find_all_spaceid_or_empty().

Diff Detail

Repository

rB Blender

Branch

T79640 (branched from master)

Build Status

Buildable 9435
Build 9435: arc lint + arc unit

Event Timeline

Philipp Oeser (lichtwerk) created this revision.Aug 10 2020, 12:33 PM

Harbormaster completed remote builds in B9435: Diff 27564.Aug 10 2020, 12:33 PM

Philipp Oeser (lichtwerk) requested review of this revision.Aug 10 2020, 12:33 PM

Philipp Oeser (lichtwerk) added a project: User Interface.Aug 10 2020, 12:33 PM

Philipp Oeser (lichtwerk) added a reviewer: Campbell Barton (campbellbarton).

Generally this is fine, I don't see a reason no to include View2D OPs here.

source/blender/windowmanager/intern/wm_keymap_utils.c
348	Why not use `WM_keymap_find_all()` with a spacetype of 0, like done for other cases here? So: WM_keymap_find_all(wm, "View2D", 0, 0);

This revision is now accepted and ready to land.Aug 10 2020, 2:53 PM

Philipp Oeser (lichtwerk) added inline comments.Aug 10 2020, 3:13 PM

source/blender/windowmanager/intern/wm_keymap_utils.c
348	Yeah, was my first intuition as well (see the patch description), but since there is a dedicated function which includes just that I thought it would be more descriptive, but you asking the question is already proof enough that it isnt :) So I guess, go with `WM_keymap_find_all`?

Julian Eisel (Severin) added inline comments.Aug 10 2020, 3:16 PM

source/blender/windowmanager/intern/wm_keymap_utils.c
348	Yeah, `_spaceid_or_empty` also isn't too helpful as a description, had to look up the definition to see what it does. It's unused now btw, I wouldn't mind if we just removed it.

Committed rBdbf5bb7af2e45ea59c7a15346a5ee989828f2d07

Revision Contents
Changeset List

Path

Size

source/

blender/

windowmanager/

intern/

wm_keymap_utils.c

5 lines

Diff 27564

View Options

source/blender/windowmanager/intern/wm_keymap_utils.c

Fix T79640: "Assign Shortcut" doesn't work for "View 2D Zoom"ClosedPublicActions