-
-
Notifications
You must be signed in to change notification settings - Fork 39.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve LAYOUT macro searching #9530
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have some style suggestions below, but they may be moot after we figure out some of the logic changes.
This looks to me like it diverges from make's behavior. Here's how make handles DEFAULT_FOLDER
:
DEFAULT_FOLDER := $$(CURRENT_KB)
# We assume that every rules.mk will contain the full default value
$$(eval include $(ROOT_DIR)/keyboards/$$(CURRENT_KB)/rules.mk)
ifneq ($$(DEFAULT_FOLDER),$$(CURRENT_KB))
$$(eval include $(ROOT_DIR)/keyboards/$$(DEFAULT_FOLDER)/rules.mk)
endif
CURRENT_KB := $$(DEFAULT_FOLDER)
If DEFAULT_FOLDER
is defined at all, not only is it used but it overrides the keyboard name and everything else. I think our basic problem here is (and always has been) that the python logic is treating them as two different boards rather than treating the one with DEFAULT_FOLDER
as an alias.
Looks like |
5690e6e
to
3c07ecf
Compare
This is working better now - |
c56c9a4
to
b24da4f
Compare
I've tested a whole bunch of different boards with all the directory structure combinations I could think of, and apart from the four that error_log is complaining about, |
b24da4f
to
8e1e737
Compare
Did a more thorough test, and went through every keyboard listed by --- a
+++ b
@@ -1,15 +1,15 @@
☒ keyboards/bpiphany/frosty_flake/frosty_flake.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
☒ keyboards/bpiphany/pegasushoof/2013/2013.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
+☒ keyboards/bpiphany/pegasushoof/2015/2015.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
+☒ keyboards/bpiphany/pegasushoof/2015/2015.h: LAYOUT_tkl_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/bpiphany/unloved_bastard/unloved_bastard.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
☒ keyboards/bpiphany/unloved_bastard/unloved_bastard.h: LAYOUT_tkl_iso: Nested layout macro detected. Matrix data not available!
-☒ clueboard/60: Missing LAYOUT() macro for 60_ansi, 60_ansi_split_bs_rshift, 60_iso
☒ keyboards/converter/usb_usb/usb_usb.h: LAYOUT_ansi: Nested layout macro detected. Matrix data not available!
☒ keyboards/converter/usb_usb/usb_usb.h: LAYOUT_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/converter/usb_usb/usb_usb.h: LAYOUT_jis: Nested layout macro detected. Matrix data not available!
⚠ dk60: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ efreet: Falling back to searching for KEYMAP/LAYOUT macros.
☒ efreet: Missing LAYOUT() macro for ortho_4x12, planck_mit
-☒ ergodash/mini: LAYOUT: Number of elements in info.json does not match! info.json:56 != LAYOUT:70
☒ keyboards/exclusive/e65/e65.h: LAYOUT_65_ansi_noblocker: Nested layout macro detected. Matrix data not available!
☒ keyboards/exclusive/e65/e65.h: LAYOUT_65_ansi_noblocker_splitbs: Nested layout macro detected. Matrix data not available!
☒ keyboards/exclusive/e65/e65.h: LAYOUT_65_iso_noblocker: Nested layout macro detected. Matrix data not available!
@@ -30,36 +30,25 @@
☒ keyboards/exclusive/e65/e65.h: LAYOUT_65_iso_7u_hhkb: Nested layout macro detected. Matrix data not available!
☒ keyboards/freyr/freyr.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
☒ keyboards/freyr/freyr.h: LAYOUT_tkl_iso: Nested layout macro detected. Matrix data not available!
-☒ handwired/co60/rev6: Missing LAYOUT() macro for 60_ansi, 60_ansi_split_bs_rshift, 60_iso, 60_hhkb
⚠ handwired/dactyl_manuform: Falling back to searching for KEYMAP/LAYOUT macros.
☒ keyboards/handwired/hnah108/hnah108.h: LAYOUT_fullsize_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/handwired/hnah108/hnah108.h: LAYOUT_fullsize_ansi: Nested layout macro detected. Matrix data not available!
⚠ handwired/jtallbean/split_65: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ handwired/traveller: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ handwired/woodpad: Falling back to searching for KEYMAP/LAYOUT macros.
-☒ helix/pico: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/pico/back: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/pico/sc: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/pico/sc/back: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/pico/sc/under: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/pico/under: LAYOUT: Number of elements in info.json does not match! info.json:50 != LAYOUT:64
-☒ helix/rev1: LAYOUT: Number of elements in info.json does not match! info.json:60 != LAYOUT:64
⚠ jm60: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ k_type: Falling back to searching for KEYMAP/LAYOUT macros.
☒ keyboards/kbdfans/kbd75/rev1/rev1.h: LAYOUT_75_iso: Nested layout macro detected. Matrix data not available!
+☒ keyboards/kbdfans/kbd75/rev2/rev2.h: LAYOUT_75_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/kc60/kc60.h: LAYOUT: Nested layout macro detected. Matrix data not available!
☒ keyboards/keebio/nyquist/nyquist.h: LAYOUT_ortho_4x12: Nested layout macro detected. Matrix data not available!
⚠ keycapsss/o4l_5x12: Falling back to searching for KEYMAP/LAYOUT macros.
☒ keycapsss/o4l_5x12: Missing LAYOUT() macro for ortho_5x12
-☒ kudox/rev1: LAYOUT: Number of elements in info.json does not match! info.json:64 != LAYOUT:66
⚠ lfkeyboards/lfk87: Falling back to searching for KEYMAP/LAYOUT macros.
☒ lfkeyboards/lfk87: Missing LAYOUT() macro for tkl_ansi, tkl_iso
⚠ lfkeyboards/mini1800: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ manta60: Falling back to searching for KEYMAP/LAYOUT macros.
-⚠ meira: Falling back to searching for KEYMAP/LAYOUT macros.
-☒ meira: Missing LAYOUT() macro for ortho_4x12
⚠ meira/featherble: Falling back to searching for KEYMAP/LAYOUT macros.
-☒ meira/featherble: Missing LAYOUT() macro for ortho_4x12
⚠ meira/promicro: Falling back to searching for KEYMAP/LAYOUT macros.
☒ meira/promicro: Missing LAYOUT() macro for ortho_4x12
☒ keyboards/phantom/phantom.h: LAYOUT_tkl_ansi: Nested layout macro detected. Matrix data not available!
@@ -67,11 +56,8 @@
☒ keyboards/phantom/phantom.h: LAYOUT_tkl_ansi_wkl: Nested layout macro detected. Matrix data not available!
☒ keyboards/phantom/phantom.h: LAYOUT_tkl_iso: Nested layout macro detected. Matrix data not available!
☒ keyboards/phantom/phantom.h: LAYOUT_tkl_iso_wkl: Nested layout macro detected. Matrix data not available!
-☒ pico/70keys: LAYOUT: Number of elements in info.json does not match! info.json:70 != LAYOUT:65
⚠ pimentoso/paddino02: Falling back to searching for KEYMAP/LAYOUT macros.
☒ keyboards/planck/ez/ez.h: LAYOUT_ortho_4x12: Nested layout macro detected. Matrix data not available!
-⚠ rgbkb/zen: Falling back to searching for KEYMAP/LAYOUT macros.
-⚠ rgbkb/zen/rev1: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ rgbkb/zen/rev2: Falling back to searching for KEYMAP/LAYOUT macros.
⚠ sirius/uni660: Falling back to searching for KEYMAP/LAYOUT macros.
☒ keyboards/sirius/unigo66/unigo66.h: LAYOUT: Nested layout macro detected. Matrix data not available! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
__attribute__((weak))
✅
Co-authored-by: Zach White <skullydazed@users.noreply.github.com>
Co-authored-by: Zach White <skullydazed@drpepper.org>
6c8d17c
to
0ab0108
Compare
Co-authored-by: Zach White <skullydazed@drpepper.org>
* 'master' of https://github.com/qmk/qmk_firmware: (178 commits) Docs: fix udev rules [CLI] Add c2json (qmk#8817) Improve LAYOUT macro searching (qmk#9530) CLI: update subcommands to use return instead of exit() (qmk#10323) Fixes small typo in docs (qmk#10515) Update personal keymap for Let's Split keyboard. (qmk#10536) [Keymap] Move my custom functions and keymaps to userspace (qmk#10502) [Keyboard] add support for ymd75 rev3 (qmk#10483) [Keyboard] Add soy20 PCB (qmk#10440) Fix for MIDI sustain effect issue (qmk#10361) format code according to conventions [skip ci] [Keyboard] Add Yugo-M Controller (qmk#10389) [Keymap] Add onekey keymap for OLED testing (qmk#10380) [Keymap] Add winterNebs keymaps (qmk#10328) [Keymap] Added 333fred 5x6_5 keymap (qmk#10272) [Keyboard] Add hannah60rgb rev.2 PCB (qmk#10287) Adding VIA support to katana60 rev2 (qmk#10442) OLED driver fixes (qmk#10377) IS31FL3741 driver fixup (qmk#10519) add info.json for XD75 keyboard (qmk#10523) ...
* upstream/master: (81 commits) [Keyboard] New keyboard - eiri (qmk#10529) [Keymap] Add niu mini dye sub keymap (qmk#10525) Clean ChibiOS platform files (qmk#10505) [Keyboard] LeftyNumpad Keyboard (qmk#10500) [Keyboard] add maja capslock indicator (qmk#10151) Fix issue introduced by PR#10404 (qmk#10559) Docs: fix udev rules [CLI] Add c2json (qmk#8817) Improve LAYOUT macro searching (qmk#9530) CLI: update subcommands to use return instead of exit() (qmk#10323) Fixes small typo in docs (qmk#10515) Update personal keymap for Let's Split keyboard. (qmk#10536) [Keymap] Move my custom functions and keymaps to userspace (qmk#10502) [Keyboard] add support for ymd75 rev3 (qmk#10483) [Keyboard] Add soy20 PCB (qmk#10440) Fix for MIDI sustain effect issue (qmk#10361) format code according to conventions [skip ci] [Keyboard] Add Yugo-M Controller (qmk#10389) [Keymap] Add onekey keymap for OLED testing (qmk#10380) [Keymap] Add winterNebs keymaps (qmk#10328) ...
* Improve LAYOUT macro searching * Apply suggestions from code review Co-authored-by: Zach White <skullydazed@users.noreply.github.com> * Adjust signature * Try to copy the makefile's handling of DEFAULT_FOLDER * Move it further up, into `info_json()` * Move it even further up so that keyboard_folder is correct * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> Co-authored-by: Zach White <skullydazed@users.noreply.github.com> Co-authored-by: Zach White <skullydazed@drpepper.org>
* Improve LAYOUT macro searching * Apply suggestions from code review Co-authored-by: Zach White <skullydazed@users.noreply.github.com> * Adjust signature * Try to copy the makefile's handling of DEFAULT_FOLDER * Move it further up, into `info_json()` * Move it even further up so that keyboard_folder is correct * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> Co-authored-by: Zach White <skullydazed@users.noreply.github.com> Co-authored-by: Zach White <skullydazed@drpepper.org>
* Improve LAYOUT macro searching * Apply suggestions from code review Co-authored-by: Zach White <skullydazed@users.noreply.github.com> * Adjust signature * Try to copy the makefile's handling of DEFAULT_FOLDER * Move it further up, into `info_json()` * Move it even further up so that keyboard_folder is correct * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> Co-authored-by: Zach White <skullydazed@users.noreply.github.com> Co-authored-by: Zach White <skullydazed@drpepper.org>
* Improve LAYOUT macro searching * Apply suggestions from code review Co-authored-by: Zach White <skullydazed@users.noreply.github.com> * Adjust signature * Try to copy the makefile's handling of DEFAULT_FOLDER * Move it further up, into `info_json()` * Move it even further up so that keyboard_folder is correct * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> Co-authored-by: Zach White <skullydazed@users.noreply.github.com> Co-authored-by: Zach White <skullydazed@drpepper.org>
* Improve LAYOUT macro searching * Apply suggestions from code review Co-authored-by: Zach White <skullydazed@users.noreply.github.com> * Adjust signature * Try to copy the makefile's handling of DEFAULT_FOLDER * Move it further up, into `info_json()` * Move it even further up so that keyboard_folder is correct * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> * Update lib/python/qmk/info.py Co-authored-by: Zach White <skullydazed@drpepper.org> Co-authored-by: Zach White <skullydazed@users.noreply.github.com> Co-authored-by: Zach White <skullydazed@drpepper.org>
Description
Trying to fix the issue I discovered in #9528 (comment), where the presence of
DEFAULT_FOLDER
causesqmk info
to check there first, even if we are as or more specific.ie:
qmk info -kb helix -l
should searchhelix.h
, thenrev2/rev2.h
. But if we wanthelix/rev1
instead, we should search there first, then fall back onDEFAULT_FOLDER
, then search everywhere else.Types of Changes
Issues Fixed or Closed by This PR
Checklist