espruino / Espruino

The Espruino JavaScript interpreter - Official Repo
http://www.espruino.com/
Other
2.76k stars 743 forks source link

added the defines to the platform file #2469

Closed brendena closed 7 months ago

brendena commented 7 months ago

Based on the topics here, I've edited the code to put the defines in the platform_config.h

Key points

How So what about this: we modify the Makefile to call scripts/build_platform_config.py $(BOARD) $(HEADERFILENAME) $(DEFINES) and then modify build_platform_config to pick up those defines from the command line and create a block like out define.

No side effects

brendena commented 7 months ago

Oooo, doesn't like (). So i escaped them and it now passes the test. I also needed to make a fix with the NULL string character in the bangle watch.

fanoush commented 7 months ago

I would never guessed the ESPR_AUTOCOMPLETE_NOT_REQUIRED does what it does from its name. what about something similar to BOARD_DEFINES_ALREADY_DEFINED? And also some comment somewhere in makefiles and/or generated header would be nice to explain why this hack exists (like "these are defines passed already in command line, helps vscode to parse them from here")

And does it actually work in vscode now?

gfwilliams commented 7 months ago

Great - thanks!

The define thing was my fault - merging now and changing to ESPR_DEFINES_ON_COMMANDLINE with a comment.

Also I'm wondering if we can pass the defines unescaped and unquoted (like is done for GCC) and save that fiddling in the Makefile (for the folks that insist on using MacOS some of the built-in unix functions are subtly different - it may be fine here but the less I can rely on them the better :)

gfwilliams commented 7 months ago

Right, I think this is all merged in now - not sure why GitHub hasn't picked it up (it usually does).