Fix some functions when using cgreen with its namespace in C++.

[TommyJC]

Hello,

Using cgreen for C++ without including using namespace cgreen in the code results in compilation errors when one attempts to use cgreen::assert_that().

I just prefixed some things with cgreen:: and added namespace cgreen { where appropriate.

The changes from this patch fixed the parts of cgreen I utilized but I haven't been able to test everything in cgreen.

Ideally I wanted to add the cgreen namespace to TestSuite in include/cgreen/internal/suite_internal.h as well but since that has namespace cgreen { later, I take it there was a good reason not to add TestSuite to the cgreen namespace.

Those who are applying using namespace cgreen to get around the compilation error should see no difference.

[thoni56]

Sorry, for the belated reply here. Cgreen has been off my radar for a while.

Thanks for this, I think this looks good, but as I'm not fully fluent in all the ins and outs of C++ that's more of an amatuers opinion.

Ideally I wanted to add the cgreen namespace to TestSuite in include/cgreen/internal/suite_internal.h as well but since that has namespace cgreen { later, I take it there was a good reason not to add TestSuite to the cgreen namespace.

You shouldn't be so confident in earlier decisions ;-) Have you tried making that change?

Perhaps @matthargett has some input on that "decision"?

[matthargett]

There were two main rules, but @strtok or @lastcraft might remember more:

1. No using statements in header files. Do not pollute implicit namespace inclusion by someone using the framework.

2. Avoid duplicating code/declarations where possible. Want want to service both languages with minimal maintenance and overhead for us and flat C users.

I think this PR works, but maybe we could use a __CGREEN_NAMESPACE_PREFIX macro that's defined to empty for non-C++ so that we don't need to duplicate all the definitions for each language? I realize we may not have done that consistently when we first did the work 16 years ago, but that's my only minor suggestion with older (never wiser ;) ) eyes.

If there's any contention about using a macro to help prefix the namespace, feel free to merge as-is.

[TommyJC]

Hi,

Just to clarify there are no "using" statements in the patch and I concur that "No using statements in header files." is a good rule to live by.

What I meant by the statement,

Those who are applying using namespace cgreen to get around the compilation error should see no difference.

is that anyone who did use using namespace cgreen in their own code to get around the issue the patch was meant to fix shouldn't see a difference.

---

As for the macro solution, I agree that that is a much cleaner and more elegant solution since it wouldn't require copying and pasting like I did with mine.

Where would that macro best be defined?

Edit: Unquoted "is that anyone who did ..." bit sentence.

[thoni56]

Using a macro to make the definitions only occur once seems cool and practical at first. But thinking a bit more, I realized that having a simple #define will not work because we cannot do

__CGREEN_NAMESPACE_PREFIXget_test_reporter...

since that will become a whole different identifier. And inserting a space

__CGREEN_NAMESPACE_PREFIX get_test_reporter...

will not work as AFAIK you are not allowed to have a space between the :: and the symbol after it.

An option would be to create a more complex macro, something like

#ifdef __cplusplus
#define CGREEN_SYMBOL(x) cgreen::x
#else
#define CGREEN_SYMBOL(x) x
#endif

But it feels this will takes away a lot of the readability of these declarations. So it comes down to, as it often does, readability or terseness/DRY.

I hope someone can either correct me or come up with a better solution because we have the same thing in legacy.h.

[thoni56]

I was ill informed and the __CGREEN_NAMESPACE_PREFIX does work. (A space is allowed after the ::...

Thanks @TommyJC !