Comment the Makefile

Also move some messages to message.mk

Makefile

@@ -2,15 +2,18 @@ ifndef VERBOSE
 .SILENT:
 endif
 
+# Allow the silent with lower caps to work the same way as upper caps
 ifdef silent
     SILENT = $(silent)
 endif
 
 ifdef SILENT
-    SUB_IS_SILENT := $(silent)
+    SUB_IS_SILENT := $(SILENT)
 endif
 
-override SILENT = false
+# We need to make sure that silent is always turned off at the top level
+# Otherwise the [OK], [ERROR] and [WARN] messags won't be displayed correctly
+override SILENT := false
 
 ON_ERROR := error_occured=1
 
@@ -26,27 +29,44 @@ ABS_STARTING_DIR := $(dir $(ABS_STARTING_MAKEFILE))
 ABS_ROOT_DIR := $(dir $(ABS_ROOT_MAKEFILE))
 STARTING_DIR := $(subst $(ABS_ROOT_DIR),,$(ABS_STARTING_DIR))
 
-PATH_ELEMENTS := $(subst /, ,$(STARTING_DIR))
-
 MAKEFILE_INCLUDED=yes
 
+# Helper function to process the newt element of a space separated path 
+# It works a bit like the traditional functional head tail
+# so the CURRENT_PATH_ELEMENT will beome the new head
+# and the PATH_ELEMENTS are the rest that are still unprocessed
 define NEXT_PATH_ELEMENT
     $$(eval CURRENT_PATH_ELEMENT := $$(firstword  $$(PATH_ELEMENTS)))
     $$(eval PATH_ELEMENTS := $$(wordlist  2,9999,$$(PATH_ELEMENTS)))
 endef
 
+# We change the / to spaces so that we more easily can work with the elements 
+# separately
+PATH_ELEMENTS := $(subst /, ,$(STARTING_DIR))
+# Initialize the path elements list for further processing
 $(eval $(call NEXT_PATH_ELEMENT))
 
+# This function sets the KEYBOARD; KEYMAP and SUBPROJECT to the correct 
+# variables depending on which directory you stand in.
+# It's really a very simple if else chain, if you squint enough, 
+# but the makefile syntax makes it very verbose. 
+# If we are in a subfolder of keyboards
 ifeq ($(CURRENT_PATH_ELEMENT),keyboards)
     $(eval $(call NEXT_PATH_ELEMENT))
     KEYBOARD := $(CURRENT_PATH_ELEMENT)
     $(eval $(call NEXT_PATH_ELEMENT))
+    # If we are in a subfolder of keymaps, or in other words in a keymap
+    # folder
     ifeq ($(CURRENT_PATH_ELEMENT),keymaps)
         $(eval $(call NEXT_PATH_ELEMENT))
         KEYMAP := $(CURRENT_PATH_ELEMENT)
+     # else if we are not in the keyboard folder itself
     else ifneq ($(CURRENT_PATH_ELEMENT),)
+        # the we can assume it's a subproject, as no other folders
+        # should have make files in them
         SUBPROJECT := $(CURRENT_PATH_ELEMENT)
         $(eval $(call NEXT_PATH_ELEMENT))
+        # if we are inside a keymap folder of a subproject
         ifeq ($(CURRENT_PATH_ELEMENT),keymaps)
             $(eval $(call NEXT_PATH_ELEMENT))
             KEYMAP := $(CURRENT_PATH_ELEMENT)
@@ -57,7 +77,8 @@ endif
 # Only consider folders with makefiles, to prevent errors in case there are extra folders
 KEYBOARDS := $(notdir $(patsubst %/Makefile,%,$(wildcard $(ROOT_DIR)/keyboards/*/Makefile)))
 
-#Compability with the old make variables
+#Compability with the old make variables, anything you specify directly on the command line
+# always overrides the detected folders
 ifdef keyboard
     KEYBOARD := $(keyboard)
 endif
@@ -71,29 +92,41 @@ ifdef keymap
     KEYMAP := $(keymap)
 endif
 
+# Uncomment these for debugging
 #$(info Keyboard: $(KEYBOARD))
 #$(info Keymap: $(KEYMAP))
 #$(info Subproject: $(SUBPROJECT))
 #$(info Keyboards: $(KEYBOARDS))
 
+
+# Set the default goal depening on where we are running make from
+# this handles the case where you run make without any arguments
 .DEFAULT_GOAL := all
 ifneq ($(KEYMAP),)
     ifeq ($(SUBPROJECT),)
+         # Inside a keymap folder, just build the keymap, with the 
+         # default subproject
         .DEFAULT_GOAL := $(KEYBOARD)-$(KEYMAP)
     else
+         # Inside a subproject keyamp folder, build the keymap
+         # for that subproject
         .DEFAULT_GOAL := $(KEYBOARD)-$(SUBPROJECT)-$(KEYMAP)
     endif
 else ifneq ($(SUBPROJECT),)
+     # Inside a subproject folder, build all keymaps for that subproject
     .DEFAULT_GOAL := $(KEYBOARD)-$(SUBPROJECT)-allkm
 else ifneq ($(KEYBOARD),)
+     # Inside a keyboard folder, build all keymaps for all subprojects
+     # Note that this is different from the old behaviour, which would
+     # build only the default keymap of the default keyboard
     .DEFAULT_GOAL := $(KEYBOARD)-allsp-allkm
 endif
 
 
-# Compare the start of the RULE_VARIABLE with the first argument($1)
+# Compare the start of the RULE variable with the first argument($1)
 # If the rules equals $1 or starts with $1-, RULE_FOUND is set to true
 #     and $1 is removed from the RULE variable
-# Otherwise the RULE_FOUND variable is set to false
+# Otherwise the RULE_FOUND variable is set to false, and RULE left as it was
 # The function is a bit tricky, since there's no built in $(startswith) function
 define COMPARE_AND_REMOVE_FROM_RULE_HELPER
     ifeq ($1,$$(RULE))
@@ -110,10 +143,12 @@ define COMPARE_AND_REMOVE_FROM_RULE_HELPER
     endif
 endef
 
+# This makes it easier to call COMPARE_AND_REMOVE_FROM_RULE, since it makes it behave like
+# a function that returns the value
 COMPARE_AND_REMOVE_FROM_RULE = $(eval $(call COMPARE_AND_REMOVE_FROM_RULE_HELPER,$1))$(RULE_FOUND)
 
 
-# Recursively try to find a match
+# Recursively try to find a match for the start of the rule to be checked
 # $1 The list to be checked
 # If a match is found, then RULE_FOUND is set to true
 # and MATCHED_ITEM to the item that was matched
@@ -127,6 +162,7 @@ define TRY_TO_MATCH_RULE_FROM_LIST_HELPER
     endif
 endef
 
+# Make it easier to call TRY_TO_MATCH_RULE_FROM_LIST
 TRY_TO_MATCH_RULE_FROM_LIST = $(eval $(call TRY_TO_MATCH_RULE_FROM_LIST_HELPER,$1))$(RULE_FOUND)
 
 define ALL_IN_LIST_LOOP
@@ -139,47 +175,70 @@ define PARSE_ALL_IN_LIST
     $$(foreach ITEM$1,$2,$$(eval $$(call ALL_IN_LIST_LOOP,$1)))
 endef
 
+# The entry point for rule parsing
+# parses a rule in the format <keyboard>-<subproject>-<keymap>-<target>
+# but this particular function only deals with the first <keyboard> part
 define PARSE_RULE
     RULE := $1
     COMMANDS :=
+    # If the rule starts with allkb, then continue the parsing from
+    # PARSE_ALL_KEYBOARDS
     ifeq ($$(call COMPARE_AND_REMOVE_FROM_RULE,allkb),true)
         $$(eval $$(call PARSE_ALL_KEYBOARDS))
+    # If the rule starts with the name of a known keyboard, then continue
+    # the parsing from PARSE_KEYBOARD
     else ifeq ($$(call TRY_TO_MATCH_RULE_FROM_LIST,$$(KEYBOARDS)),true)
         $$(eval $$(call PARSE_KEYBOARD,$$(MATCHED_ITEM)))
+    # Otherwise use the KEYBOARD variable, which is determined either by
+    # the current directory you run make from, or passed in as an argument
     else ifneq ($$(KEYBOARD),)
         $$(eval $$(call PARSE_KEYBOARD,$$(KEYBOARD)))
     else
         $$(info make: *** No rule to make target '$1'. Stop.)
+        # Notice the tab instead of spaces below!
 		exit 1
     endif
 endef
 
 # $1 = Keyboard
+# Parses a rule in the format <subproject>-<keymap>-<target>
+# the keyboard is already known when entering this function
 define PARSE_KEYBOARD
     CURRENT_KB := $1
     # A subproject is any keyboard subfolder with a makefile
     SUBPROJECTS := $$(notdir $$(patsubst %/Makefile,%,$$(wildcard $(ROOT_DIR)/keyboards/$$(CURRENT_KB)/*/Makefile)))
+    # if the rule starts with allsp, then continue with looping over all subprojects
     ifeq ($$(call COMPARE_AND_REMOVE_FROM_RULE,allsp),true)
         $$(eval $$(call PARSE_ALL_SUBPROJECTS))
+    # A special case for matching the defaultsp (default subproject)
     else ifeq ($$(call COMPARE_AND_REMOVE_FROM_RULE,defaultsp),true)
         $$(eval $$(call PARSE_SUBPROJECT,defaultsp))
+    # If the rule starts with the name of a known subproject
     else ifeq ($$(call TRY_TO_MATCH_RULE_FROM_LIST,$$(SUBPROJECTS)),true)
         $$(eval $$(call PARSE_SUBPROJECT,$$(MATCHED_ITEM)))
+    # Try to use the SUBPROJECT variable, which is either determined by the
+    # directory which invoked make, or passed as an argument to make
     else ifneq ($$(SUBPROJECT),)
         $$(eval $$(call PARSE_SUBPROJECT,$$(SUBPROJECT)))
-    else 
 	# If there's no matching subproject, we assume it's the default
 	# This will allow you to leave the subproject part of the target out
+    else 
         $$(eval $$(call PARSE_SUBPROJECT,defaultsp))
     endif
 endef
 
+# if we are going to compile all keyboards, match the rest of the rule
+# for each of them
 define PARSE_ALL_KEYBOARDS
     $$(eval $$(call PARSE_ALL_IN_LIST,PARSE_KEYBOARD,$(KEYBOARDS)))
 endef
 
 # $1 Subproject
+# When entering this, the keyboard and subproject are known, so now we need
+# to determine which keymaps are going to get compiled
 define PARSE_SUBPROJECT
+    # If we want to compile the default subproject, then we need to 
+    # include the correct makefile to determine the actual name of it
     ifeq ($1,defaultsp)
         SUBPROJECT_DEFAULT=
         $$(eval include $(ROOT_DIR)/keyboards/$$(CURRENT_KB)/Makefile)
@@ -188,39 +247,54 @@ define PARSE_SUBPROJECT
         CURRENT_SP := $1
     endif
     # If current subproject is empty (the default was not defined), and we have a list of subproject
-    # then make all
+    # then make all of them
     ifeq ($$(CURRENT_SP),)
         ifneq ($$(SUBPROJECTS),)
             CURRENT_SP := allsp
          endif
     endif
+    # The special allsp is handled later
     ifneq ($$(CURRENT_SP),allsp) 
+        # get a list of all keymaps
         KEYMAPS := $$(notdir $$(patsubst %/.,%,$$(wildcard $(ROOT_DIR)/keyboards/$$(CURRENT_KB)/keymaps/*/.)))
         ifneq ($$(CURRENT_SP),)
+            # if the subproject is defined, then also look for keymaps inside the subproject folder
             SP_KEYMAPS := $$(notdir $$(patsubst %/.,%,$$(wildcard $(ROOT_DIR)/keyboards/$$(CURRENT_KB)/$$(CURRENT_SP)/keymaps/*/.)))
             KEYMAPS := $$(sort $$(KEYMAPS) $$(SP_KEYMAPS))
         endif
+        # if the rule after removing the start of it is empty (we haven't specified a kemap or target)
+        # compile all the keymaps
         ifeq ($$(RULE),)
             $$(eval $$(call PARSE_ALL_KEYMAPS))
+        # The same if allkm was specified
         else ifeq ($$(call COMPARE_AND_REMOVE_FROM_RULE,allkm),true)
             $$(eval $$(call PARSE_ALL_KEYMAPS))
+        # Try to match the specified keyamp with the list of known keymaps
         else ifeq ($$(call TRY_TO_MATCH_RULE_FROM_LIST,$$(KEYMAPS)),true)
             $$(eval $$(call PARSE_KEYMAP,$$(MATCHED_ITEM)))
+        # Otherwise try to match the keymap from the current folder, or arguments to the make command
         else ifneq ($$(KEYMAP),)
             $$(eval $$(call PARSE_KEYMAP,$$(KEYMAP)))
         else
+            # Otherwise something is wrong with the target
+            # Try to give as much information as possible of what it it was trying to do
             ifeq ($$(CURRENT_SP),)
                 $$(info make: *** No rule to make target '$$(CURRENT_KB)-$$(RULE)'. Stop.)
             else
                 $$(info make: *** No rule to make target '$$(CURRENT_KB)-$$(CURRENT_SP)-$$(RULE)'. Stop.)
             endif
+            # Notice the tab instead of spaces below!
 			exit 1
         endif
     else
+        # As earlier mentione,d when allsb is specified, we call our self recursively
+        # for all of the subprojects
         $$(eval $$(call PARSE_ALL_IN_LIST,PARSE_SUBPROJECT,$(SUBPROJECTS)))
     endif
 endef
 
+# If we want to parse all subprojects, but the keyboard doesn't have any, 
+# then use defaultsp instead
 define PARSE_ALL_SUBPROJECTS
     ifeq ($$(SUBPROJECTS),)
         $$(eval $$(call PARSE_SUBPROJECT,defaultsp))
@@ -230,27 +304,36 @@ define PARSE_ALL_SUBPROJECTS
 endef
 
 # $1 Keymap
+# This is the meat of compiling a keyboard, when entering this, everything is known
+# keyboard, subproject, and keymap
+# Note that we are not directly calling the command here, but instead building a list,
+# which will later be processed
 define PARSE_KEYMAP
     CURRENT_KM = $1
     # The rest of the rule is the target
     # Remove the leading "-" from the target, as it acts as a separator
     MAKE_TARGET := $$(patsubst -%,%,$$(RULE))
+    # We need to generate an unique indentifer to append to the COMMANDS list
     COMMAND := COMMAND_KEYBOARD_$$(CURRENT_KB)_SUBPROJECT_$(CURRENT_SP)_KEYMAP_$$(CURRENT_KM)
     COMMANDS += $$(COMMAND)
+    # If we are compiling a keyboard without a subproject, we want to display just the name
+    # of the keyboard, otherwise keyboard/subproject
     ifeq ($$(CURRENT_SP),)
         KB_SP := $(CURRENT_KB)
     else
         KB_SP := $(CURRENT_KB)/$$(CURRENT_SP)
     endif
+    # Format it in bold
     KB_SP := $(BOLD)$$(KB_SP)$(NO_COLOR)
+    # Specify the variables that we are passing forward to submake
     MAKE_VARS := KEYBOARD=$$(CURRENT_KB) SUBPROJECT=$$(CURRENT_SP) KEYMAP=$$(CURRENT_KM)
     MAKE_VARS += VERBOSE=$(VERBOSE) COLOR=$(COLOR)
+    # And the first part of the make command
     MAKE_CMD := $$(MAKE) -r -R -C $(ROOT_DIR) -f build_keyboard.mk $$(MAKE_TARGET)
-    MAKE_MSG := Making $$(KB_SP) with keymap $(BOLD)$$(CURRENT_KM)$(NO_COLOR)
+    # The message to display
-    ifneq ($$(MAKE_TARGET),)
+    MAKE_MSG := $$(MSG_MAKE_KB)
-        MAKE_MSG += and target $(BOLD)$$(MAKE_TARGET)$(NO_COLOR)
+    # We run the command differently, depending on if we want more output or not
-    endif
+    # The true version for silent output and the false version otherwise
-    MAKE_MSG_FORMAT := $(AWK) '{ printf "%-118s", $$$$0;}'
     COMMAND_true_$$(COMMAND) := \
         printf "$$(MAKE_MSG)" | \
         $$(MAKE_MSG_FORMAT); \
@@ -267,10 +350,14 @@ define PARSE_KEYMAP
         $$(MAKE_CMD) $$(MAKE_VARS) SILENT=false;
 endef
 
+# Just parse all the keymaps for a specifc keyboard
 define PARSE_ALL_KEYMAPS
     $$(eval $$(call PARSE_ALL_IN_LIST,PARSE_KEYMAP,$$(KEYMAPS)))
 endef
 
+# Set the silent mode depending on if we are trying to compile multiple keyboards or not
+# By default it's on in that case, but it can be overriden by specifying silent=false 
+# from the command line
 define SET_SILENT_MODE
     ifdef SUB_IS_SILENT
         SILENT_MODE := $(SUB_IS_SILENT)
@@ -291,9 +378,12 @@ SUBPROJECTS := $(notdir $(patsubst %/Makefile,%,$(wildcard ./*/Makefile)))
 .PHONY: $(SUBPROJECTS)
 $(SUBPROJECTS): %: %-allkm 
 
+# Let's match everything, we handle all the rule parsing ourselves
 .PHONY: %
 %: 
+	# Check if we have the CMP tool installed
 	cmp --version >/dev/null 2>&1; if [ $$? -gt 0 ]; then printf "$(MSG_NO_CMP)"; exit 1; fi;
+	# Check if the submodules are dirty, and display a warning if they are
 	git submodule status --recursive 2>/dev/null | \
 	while IFS= read -r x; do \
 		case "$$x" in \
@@ -303,14 +393,20 @@ $(SUBPROJECTS): %: %-allkm
 	done
 	$(eval $(call PARSE_RULE,$@))
 	$(eval $(call SET_SILENT_MODE))
+	# Run all the commands in the same shell, notice the + at the first line
+	# it has to be there to allow parallel execution of the submake
+	# This always tries to compile everything, even if error occurs in the middle
+	# But we return the error code at the end, to trigger travis failures
 	+error_occured=0; \
 	$(foreach COMMAND,$(COMMANDS),$(RUN_COMMAND)) \
 	if [ $$error_occured -gt 0 ]; then printf "$(MSG_ERRORS)" & exit $$error_occured; fi
 	
 
+# All should compile everything
 .PHONY: all
 all: all-keyboards 
 
+# Define some shortcuts, mostly for compability with the old syntax
 .PHONY: all-keyboards
 all-keyboards: allkb-allsp-allkm
 
@@ -318,6 +414,7 @@ all-keyboards: allkb-allsp-allkm
 all-keyboards-defaults: allkb-allsp-default
 
 
+# Generate the version.h file
 GIT_VERSION := $(shell git describe --abbrev=6 --dirty --always --tags 2>/dev/null || date +"%Y-%m-%d-%H:%M:%S")
 BUILD_DATE := $(shell date +"%Y-%m-%d-%H:%M:%S")
 $(shell echo '#define QMK_VERSION "$(GIT_VERSION)"' > $(ROOT_DIR)/quantum/version.h)

message.mk

@@ -31,7 +31,7 @@ PRINT_ERROR_PLAIN = ($(SILENT) ||printf " $(ERROR_STRING)" | $(AWK_STATUS)) && $
 PRINT_WARNING_PLAIN = ($(SILENT) || printf " $(WARN_STRING)" | $(AWK_STATUS)) && $(TAB_LOG_PLAIN)
 PRINT_OK = $(SILENT) || printf " $(OK_STRING)" | $(AWK_STATUS)
 BUILD_CMD = LOG=$$($(CMD) 2>&1) ; if [ $$? -gt 0 ]; then $(PRINT_ERROR); elif [ "$$LOG" != "" ] ; then $(PRINT_WARNING); else $(PRINT_OK); fi;
-MSG_NO_CMP = $(ERROR_COLOR)Error:$(NO_COLOR)$(BOLD) cmp command not found, please install diffutils\n$(NO_COLOR)
+MAKE_MSG_FORMAT = $(AWK) '{ printf "%-118s", $$0;}'
 
 # Define Messages
 # English
@@ -60,3 +60,12 @@ MSG_SUBMODULE_DIRTY = $(WARN_COLOR)WARNING:$(NO_COLOR)\n \
 	git submodule update --init --recursive$(NO_COLOR)\n\n\
 	You can ignore this warning if you are not compiling any ChibiOS keyboards,\n\
 	or if you have modified the ChibiOS libraries yourself. \n\n
+MSG_NO_CMP = $(ERROR_COLOR)Error:$(NO_COLOR)$(BOLD) cmp command not found, please install diffutils\n$(NO_COLOR)
+
+define GENERATE_MSG_MAKE_KB
+    MSG_MAKE_KB_ACTUAL := Making $$(KB_SP) with keymap $(BOLD)$$(CURRENT_KM)$(NO_COLOR)
+    ifneq ($$(MAKE_TARGET),)
+        MSG_MAKE_KB_ACTUAL += and target $(BOLD)$$(MAKE_TARGET)$(NO_COLOR)
+    endif
+endef
+MSG_MAKE_KB = $(eval $(call GENERATE_MSG_MAKE_KB))$(MSG_MAKE_KB_ACTUAL)