Thanks for this @giuliomoro
Just a quick comment: for the new files you could add copyright information, if you'd like (like here, for example)

Thanks for this @giuliomoro

thanks for having a look!

Just a quick comment: for the new files you could add copyright information, if you'd like (like here, for example)

Is it OK not to have it in there? I feel like those things tend to get outdated over time and I am never sure what their purpose is ... The commit history will tell who the author(s) have been more in detail.

I feel like those things tend to get outdated over time and I am never sure what their purpose is ... The commit history will tell who the author(s) have been more in detail.

git author does not necessarily tell you who is the actual author of the code, and it's not a replacement for a copyright claim which as i understand it is primarily to give a person or persons legal standing to sue in the case of a license violation. that said, we don't have a policy as far as i know regarding this. in my experience if the author of a piece of code wishes to name either themself, James McCartney, or the Free Software Foundation as the copyright holder that's generally acceptable, so long as they are ok with it being licensed with GPL3 or another compatible license. AFAIK it's not ok to remove copyright notices without explicit permission of the author.

also, this is the most co-authors (5) i've ever seen on a commit, cool: 84f24e2

thanks @brianlheim

format the sc code so it is indented in tabs,

and this cannot be done with clang-format, right? Any chance such a feature could be added to it?

indent all the new cmake code at 4 spaces,

even when the surrounding code uses tabs (e.g.: lines 148:153 of server/scsynth/CMakeLists.txt) ? I understand this should hold, but I thought I should ask.

replace the try_or_retry macro in XenomaiLock.cpp with a real function

this would lead to plenty of code duplication, is that really preferable to the current situation?

thanks @brianlheim

and this cannot be done with clang-format, right? Any chance such a feature could be added to it?

good luck getting llvm devs to accept that PR. :)

you can do this in the SCIDE. no plans right now for having a linter/formatter for sc code but it would be nice.

even when the surrounding code uses tabs (e.g.: lines 148:153 of server/scsynth/CMakeLists.txt) ? I understand this should hold, but I thought I should ask.

yes

this would lead to plenty of code duplication, is that really preferable to the current situation?

i don't understand. there will not be much duplicated code if you replace the macro with a function. and yes, almost anything would be better than that macro.

@brianlheim you can do this in the SCIDE. no plans right now for having a linter/formatter for sc code but it would be nice.

Does this mean:

• open the file in SCIDE
• select all
• go to Edit->Autoindent Line or Region
• save file
  ?

I tried the above for all the files we created/modified and this is the outcome: 9be6c8c

Most of the changes look OK, but

• I am not sure if that's all that's needed
• some of the lines we didn't change were affected in the process

yeah, the sc code changes look good now, thanks! FYI, the cmake code in the FindX.cmake modules is still indented at 2 instead of 4 spaces

@brianlheim FYI, the cmake code in the FindX.cmake modules is still indented at 2 instead of 4 spaces

Thanks, fixed that.

@elgiano would you have time to act on the comments by @telephon please?

EDIT: I mean, I am not sure what they are referring to.

EDIT: I mean, I am not sure what they are referring to.

They refer to the sclang classes added in this PR.

thanks again @giuliomoro . i've opened a 3rd and final PR to your branch here: https://github.com/BelaPlatform/supercollider/pull/91/files

i've opened a 3rd and final PR to your branch here:

merged

Re: BelaScope using a C API instead of C++, is this enough? https://github.com/BelaPlatform/supercollider/tree/topic/bela-scope-c

thanks @giuliomoro !

Re: BelaScope using a C API instead of C++, is this enough? https://github.com/BelaPlatform/supercollider/tree/topic/bela-scope-c

unfortunately i don't think that's enough, because BelaScope itself is still a C++ class using C++ language features (new, delete, classes, etc.). let's leave this discussion until after the PR is merged please, it is too difficult to keep track of everything going on here. please update the readme when you have time and then i'll approve and merge this :)

OK thanks, I will ping you when done.

unfortunately i don't think that's enough

Agreed. Generally, just use a plain C struct for the public data members that the plugins should see. Then inherit from this struct for the actual private implementation. All operations on the struct that go beyond simple member access have to happen via plugin API methods.

In your case that would probably be:

// in the plugin interface header:
struct BelaScope {
    float* buffer;
    uint32_t maxChannels;
    uint32_t bufferSamples;
    int touched; // no bool!
};

// in the implementation .cpp file
class BelaScopeImp : public BelaScope {
public:
    // methods
private:
    Scope scope;
};

// in InterfaceTable
void (fBelaScopeLogBuffer*)(World *, BelaScope *);

BTW, if you need to add several API methods to the interface table, it might make sense to group them in a seperate Bela interface and put that in the SC InterfaceTable.

struct BelaInterface {
    void (fScopeLogBuffer*)(World *, BelaScope *);
    // other methods...
};

// and in InterfaceTable
BelaInterfaceTable *mBelaInterface;

@brianlheim here's the updated README. https://github.com/BelaPlatform/supercollider/blob/bela-ready-to-merge/README_BELA.md

New code is tested and ready to go.
I'd recommend squashing before merging, let me know if you want me to do that, please.

@giuliomoro I just looked over the readme and it looks good. thanks for that, and for testing the new changes too! yes, could you please squash it?

pushed!

also, this is the most co-authors (5) i've ever seen on a commit, cool: 84f24e2

that's because you haven't seen the six in b773117 !

thanks to everyone who worked on this! Bela is now in mainline SC 🎉

thanks @brianlheim for taking so much time to go through this and improve it a fair bit in the process and thanks everyone else who has contributed to it with precious code and/or precious reviews.