[Feature] Compilation warning if both keymap.json and keymap.c exist

[elpekenin]

Description

Small changes so that we look for both files, instead of not searching for C file when JSON exists.
With this information we can check if both were found to throw a warning, and then procede to compile JSON.
This prevents people(aka happened to me a few weeks ago) from going insane if working on a folder where json2c has been used and JSON hasn't been deleted.

Tested:

• JSON- or C-only setups work as usual
• Having none of them throws the regular "no keymap found" error
• Having both shows the warning and compiles the JSON file

⚠️ This is my 1st time ever writing MK, it can probably be cleaned/simplified, but I've struggled enough getting it to work.

Types of Changes

• Core
• Bugfix
• New feature
• Enhancement/optimization
• Keyboard (addition or update)
• Keymap/layout/userspace (addition or update)
• Documentation

Issues Fixed or Closed by This PR

• Fixes [Feature Request] Keymap compilation warning #19467

Checklist

• My code follows the code style of this project: C, Python
• I have read the PR Checklist document and have made the appropriate changes.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• I have added tests to cover my changes.
• I have tested the changes and verified that they work and don't break anything (as well as I can manage).

[elpekenin]

Dumb bot remove the tag...

I've updated to remove the ifeq which will always be true as the KEYMAP_PATH variable isn't set on build_json.mk anymore.

However, this made me realize that my changes will pull mk files from the keymap folder on keymap.json compilations while previous code didn't do such thing... No idea if it could cause any issues and was made like that intentionally or not

[tzarc]

Seems fine.

Stale

[zvecr]

However, this made me realize that my changes will pull mk files from the keymap folder on keymap.json compilations while previous code didn't do such thing... No idea if it could cause any issues and was made like that intentionally or not

If this is rules.mk files adjacent to keymap.json, this seems fine to me. Though I thought that was already happening with the block,

    # Load the keymap-level rules.mk if exists
    -include $(KEYMAP_PATH)/rules.mk

[elpekenin]

Oh yeah, i definitely overlooked at it, and didnt research further. As i said before, im about as noob as you can be with makefile.... But it is a bit confusing because the JSON keymaps include later on, while the C ones do that when checking the path:

ifneq ("$(wildcard $(MAIN_KEYMAP_PATH_1)/keymap.c)","")
    -include $(MAIN_KEYMAP_PATH_1)/rules.mk
    KEYMAP_C := $(MAIN_KEYMAP_PATH_1)/keymap.c
    KEYMAP_PATH := $(MAIN_KEYMAP_PATH_1)

vs

ifneq ("$(wildcard $(MAIN_KEYMAP_PATH_5)/keymap.json)","")
    KEYMAP_JSON := $(MAIN_KEYMAP_PATH_5)/keymap.json
    KEYMAP_JSON_PATH := $(MAIN_KEYMAP_PATH_5)

Perhaps one of them could be changed for consistency

PD: JSON checks 5 >> 1, while C does 1 >> 5, not sure if that could also be changed, maybe breaks some hierarchy?

[zvecr]

Its done later, as the searching is skipped when running compile_configurator_json from the python world. While it shouldnt do any harm, it probably should be updated to not set KEYMAP_PATH (and/or tested too).

[tzarc]

@elpekenin was zvecr's query handled?

[elpekenin]

@elpekenin was zvecr's query handled?

Im not sure what im supposed to change, to be honest... If both files (c and json) exist, KEYMAP_JSON_PATH and KEYMAP_PATH will be the same. Then, since they were splitted into 2 different variables (to be able to warn after finding for both) from KEYMAP_PATH, i copy the value (just for extra safety tho, it shouldn't be needed). So, new code should work fine (and in the exact same way as old one), and as far as my testing back in the day went, it does

Perhaps you are refering to the consistency i asked about... Given my lack of knowledge of the compile_configurator_json pipeline -and the whole makefile thing- i opted not doing anything about it, but the changes on the PR are good to go from my side

[github-actions]

Thank you for your contribution!
This pull request has been automatically marked as stale because it has not had activity in the last 45 days. It will be closed in 30 days if no further activity occurs. Please feel free to give a status update now, or re-open when it's ready.
For maintainers: Please label with bug, awaiting review, breaking_change, in progress, or on hold to prevent the issue from being re-flagged.

[github-actions]

Thank you for your contribution!
This pull request has been automatically closed because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, or re-open when it's ready.
// [stale-action-closed]