Fix generating of modules list for Windows lineends

marcelstoer / docker-nodemcu-build

Docker image to build NodeMCU firmware for the ESP8266 on your machine

https://hub.docker.com/r/marcelstoer/nodemcu-build/

MIT License

129 stars 63 forks source link

Fix generating of modules list for Windows lineends #41

Closed HHHartmann closed 5 years ago

HHHartmann commented 5 years ago

if the sourcecode is checked out under windows the linenedings are windows style. This patch collects and replaces the modules list even then.

marcelstoer commented 5 years ago

Thanks, don't have Windows to test here. What I'm wondering:

If this is indeed a bug (your reasoning is logical), then why hasn't it been reported before? With over 9k pulls this image sure must have run on a number of Windows boxes.
Did you already confirm that it still runs fine on Unix/Mac?

HHHartmann commented 5 years ago

I don't have a Mac/Linux here, so didn't test it there. I will convert the files to UNX line ends and have a look.

As to why this hasn't been reported:

People don't report bugs
the Bug is not a breaker but just a missing feature which isn't documented AFAICS.

marcelstoer commented 5 years ago

I don't have a Mac/Linux here

No worries, I'll test on a Mac.

...just a missing feature which isn't documented AFAICS

True, just wanted reach feature-parity with my cloud builder (isn't documented there either)

HHHartmann commented 5 years ago

But definitely is a cool feature, here and there. I will also add int/float to the description. Would it make sense to store this Information outside the img file also? Like either have a longish filename or add a readme aside the image containing the complete info as shown on bootup?

Have a separate PR for that?

marcelstoer commented 5 years ago

Would it make sense to store this Information outside the img file also?

I personally see very little value in that. It has never been requested in the last three years either. That being said if you later put up a separate PR with code that would create a <img-filename>.txt I would definitely consider it. Long file names, no.