feat(qmk): made the install script safer on rerun. #81
Conversation
AkiroPhi
left a comment
AkiroPhi
left a comment
There was a problem hiding this comment.
I just checked the code shortly, i neither read the code entirely neither try it to run for now
| echo " # -b: build the newly created keymap (after you edited the config)" | ||
| echo " # -f: build the newly created keymap (after you edited the config) then flash" | ||
| echo " # -c: compile the newly created keymap (after you edited the config)" | ||
| echo " # -f: compile the newly created keymap (after you edited the config) then flash" |
There was a problem hiding this comment.
May be you can do something like
-f: flash the newly created keymap and compile it (if needed ?) after you edited the config
At first reading, we can misunderstand the use of -f and precise flash after -f is more clear according to me
| "Recompile and flash keymap with the QMK CLI") | ||
| cd "$QMK_PATH" | ||
| make "$keyboard_name":arsenik:flash | ||
| return 1 | ||
| ;; | ||
| "Recompile and flash keymap with \`make\`") | ||
| qmk flash -kb $keyboard_name -km arsenik | ||
| return 1 | ||
| ;; | ||
| "Abort") | ||
| echo "Aborting." | ||
| return 1 | ||
| ;; | ||
| *) | ||
| echo "Unknown command chosen. Aborting." | ||
| return 1 | ||
| ;; |
There was a problem hiding this comment.
Should be the return code used ? If so why returning 1when aborting, and doing make, and doing qmk flash ?
Is it possible to return the code of previous command ?
There was a problem hiding this comment.
Yeah, I wanted the function to be able to tell if we should still reinstall the keymap after this case statement (see this comment), but it’s kinda dumb. If you have a better idea to return a status from this function, I’m all ears.
| [ -d "$arsenik_folder" ] && handle_keymap_conflict "$arsenik_folder" | ||
|
|
There was a problem hiding this comment.
The return code of handle_keymap_conflict is not used.
Is it ok ? commit seems to prevent rerun but aborting don’t abort the execution ?
There was a problem hiding this comment.
It is but in a pretty roundabout way. Basically, the set -euo pipefail at the start of the file makes the whole script exit with an error if one command returns an error. So if the exit code from handle_keymap_conflict is not 0, then the script will exit out (and thus, not reinstall the keymap).
I thought it was a bit dirty but maybe fine ? But if you don’t get it then it’s not clear enough and should be fixed.
No description provided.