arduino / arduino-builder

A command line tool for compiling Arduino sketches
GNU General Public License v2.0
458 stars 114 forks source link

add #line directive to additional source files #325

Closed slady closed 3 years ago

slady commented 5 years ago

improvement for https://github.com/arduino/arduino-builder/issues/323

slady commented 5 years ago

To your first doubt: I double checked the contents of the additional files list, it can contain files with these extensions: .h .c .hpp .hh .cpp .s Hope all of them can start with the #line directive.

To your second doubt: Reading the whole file into memory seems a little bit odd and superfluous, but it was already in the source code long before my changes took place. My change only added one little line to the contents of the whole file. Hope this did not blow it up too much.

oschonrock commented 4 years ago

+1 This would be super useful.

I presume this would "automatically" be part of ardino-cli ? Which is what I am using, and what people who care about correct error messages could be likely to use?

Any way to move this along? Now has conflicts?

slady commented 4 years ago

the bad news is they had changed the master, without incorporating my changes, so now we have conflicts... :-(

the good news is that my friend already rewrote my code to the new base :-)

we can try to re-apply his changes, but I highly doubt those would get merged into the main base... :-/

so our only hope is to fork the project and take control over it! ;-)

oschonrock commented 4 years ago

Shame, it's a pretty trivial change, which is not very invasive and very useful .

Is there a pull request for the updated version against current master?

matthijskooijman commented 4 years ago

I believe this code was moved to the arduino-cli repo, so any new PR should be created there. There's also quite some movement in that repo, so I'm hopeful it can be merged in there.

slady commented 4 years ago

I am not active on arduino any more, since I am writing a game for 8-bit computer ZX Spectrum right now along with a new IO game online game

however I will let my friend know that you are interested in this kind of change so he can step in and take over the hassle of communication with the arduino development team

oschonrock commented 4 years ago

I am running arduino-cli V 0.10.0, which includes this line:

https://github.com/arduino/arduino-cli/blob/ec5c3ed105b32c5654fd60131a667f8557b196d5/arduino/builder/sketch.go#L242

Which looks to me like it should be doing what we want.

So this actually becomes a different case altogether? ie, a bug/issue, why is it not working on my code. I will investigate. Could well be to do with arduino-cli expecting my "extra files" to have a different extension or something like that.

Anyway, I think we should close this issue, it can never be merged. And "in spirit" at least, it has been included in arduino-cli.

Agree?

matthijskooijman commented 4 years ago

That line applies to the merged .ino files (e.g. the MainFile and the OtherSketchFiles, which together are all .ino files). This issue is about the AdditionalFiles (maybe not so brilliant terminology when you look at it like this :-p), which are copied verbatim a few lines further down.

oschonrock commented 4 years ago

@matthijskooijman Yeah, I just kind of figured that out too. Arduino does do some really weird preprocessing, coming from a CPP background.

We just need a single line inserted at the beginning of each "additional file" copy. So this should not be too hard. I know no Go at all, but I think even I can manage that with a bit of "copy/paste coding".

Will have a "Go"... ;-) and make a pull request.

Thanks

oschonrock commented 4 years ago

It was a one-liner. Here is the pull request: https://github.com/arduino/arduino-cli/pull/707 Hope it gets through

CLAassistant commented 3 years ago

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

per1234 commented 3 years ago

Closing as superseded by https://github.com/arduino/arduino-cli/pull/1224. Thanks everyone for your contributions!