Turn signals.py into Signal<Combiner, R, A...> variadic template

[aledomu]

My C++ skills were a bit rusty and I chose my favourite DAW to practice on. I thought this could make the code a bit more accesible for external contributors. I hope you find this useful.

[pauldavisthefirst]

This is a noble and worthy effort.

However, we have to manage multiple code bases now (Mixbus and LiveTrax), and we can't really consider applying it without an accompanying script that can be run to automatically edit additional files present in those code bases too. I assume you pobably used such a thing when working on this?

[aledomu]

I just used regex pattern matching for batch replacing occurrences across all files. I did it in a bit of an ad-hoc way because there were stylistic differences. So there is no fully automated script, but I can explain the process.

For the first commit the basic pattern was this for users of SignalN:

• Signal0<R> => Signal<R>
• Signal1<R, A1> => Signal<R, A1>
• Signal2<R, A1, A2> => Signal<R, A1, A2>
• ...

Pretty simple here. It is essentially the same as SignalN => Signal.

There was a single SignalN instance where it specified a non-default combiner. In that case the replacement was SignalN<R, A1, A2, ..., Combiner> => SignalWithCombiner<Combiner, R, A1, A2, ...>.

Now with the stylistic differences.

Some SignalN instances (let's take Signal2 invocations as an example) where like this:

• Signal2 <R, A1, A2> (notice the space before the template instance brackets)
• Signal2<R,A1,A2>
• Signal2<R, A1, A2<> > (the last template argument was itself a template instance so a space at the end is sometimes inserted to separate the end brackets)
• A combination of the above

With these considerations, I started batch replacing each SignalN in cascade. Before compiling to check that the types matched (there is no behaviour change involved), I assured that no SignalN (for each N) was left in the project. Here the issues where about not getting the variadic template and the template specialization right.

Then I replaced Signal<R, A1, A2, ...> with Signal<R(A1, A2, ...)> and SignalWithCombiner<Combiner, R, A1, A2> with SignalWithCombiner<Combiner, R(A1, A2, ...)>. This was trickier and more error-prone with the stylistic differences, but I did it this way so if you did not like this alternative form I could just reverse this change in the instances. However if you do like it, I recommend that I merge both commits so you can more easily batch replace these patterns in those other codebases, considering the style differences:

• Signal0<R> => Signal<R()>
• Signal1<R, A1> => Signal<R(A1)>
• Signal2<R, A1, A2> => Signal<R(A1, A2)>
• ...

There is another path though. I can separate the replacement of SignalN instances in a different commit and make it optional by introducing type aliases for SignalN using Signal but not SignalWithCombiner. This does not exclude merging the first and second commit as I said before. However I still suggest that in the end those non-variadic instances should be eliminated in the codebase that is not publicly available, because those type aliases would not be useful for anything else than backwards compatibility within the project codebase itself.

I'm open to suggestions.

[aledomu]

I got this in a mergeable state for users of the old SignalN classes without requiring them to upgrade instantly, and with no requirement for C++17. I extracted that bit out for another PR though.

[x42]

I've written a simple script to convert all signals:

#!/bin/bash

for file in `ag -l 'PBD::Signal0' libs gtk2_ardour` ; do
	sed -i'' 's/PBD::Signal0<\([^>]*\)>/PBD::Signal<\1()>/' $file
done

patch -p1 << EOF
diff --git a/libs/ardour/ardour/io.h b/libs/ardour/ardour/io.h
index e4cef915ab..65d7c91584 100644
--- a/libs/ardour/ardour/io.h
+++ b/libs/ardour/ardour/io.h
@@ -181,6 +181,6 @@ public:
 	 *  the change from happening.
 	 */
-	PBD::Signal1<bool, ChanCount, BoolCombiner> PortCountChanging;
+	PBD::SignalWithCombiner<BoolCombiner, bool(ChanCount)> PortCountChanging;
 
 	static PBD::Signal1<void, ChanCount> PortCountChanged; // emitted when the number of ports changes
EOF
 

#apply some special cases, where the return value is templated
for file in `ag -l 'PBD::Signal[1-9]+' libs gtk2_ardour`; do
	sed -i'' 's/PBD::Signal[1-9] *<\([^,]*<[^>]*>\), *\(.*\) *>/PBD::Signal<\1(\2)>/' $file
done

for file in `ag -l 'PBD::Signal[1-9]+' libs gtk2_ardour`; do
	sed -i'' 's/PBD::Signal[1-9] *<\([^,]*\), *\(.*\) *>/PBD::Signal<\1(\2)>/' $file
done

[x42]

Rebased and merged as 9.0-pre0-243-g0ade0b2212

I also ran the script to convert the codebase to use new Signal<> API, and removed the old one.