Fix sphinxrenderer.py variable and function visitors for C#

[rogerbarton]

Fixes #730 and rogerbarton/sphinx-csharp#5

C# requires slightly different treatment when visiting variables and functions.

[michaeljones]

Thank you for contributing. Would you be able to provide an example in the 'specific examples' section of the documentation that covers this? That is the closest thing we have to regression checks at the moment :)

[rogerbarton]

Yes I can have a look at adding some specific examples. I started adding some test examples in the sphinx-csharp repo but it makes more sense to have them here. The c# domain really needs some more testing/verification and docs. I will try to work on this...

Should I add a cs subfolder in examples/specific?

[jakobandersen]

Should I add a cs subfolder in examples/specific?

These tests will probably be revamped anyway, so for now I suggest not bothering with an extra folder, but prefixing the files with cs_ (similar to some of the current C and C++ tests).
For the actual addition, there are quite a few places to change. I think https://github.com/michaeljones/breathe/pull/723/files may provide an ok view of where changes are needed.

[jakobandersen]

I started adding some test examples in the sphinx-csharp repo but it makes more sense to have them here. The c# domain really needs some more testing/verification and docs. I will try to work on this...

As the C# domain can be used without Breathe I recommend leaving tests in that repo, and then add Breathe-specific tests here.

[vermeeren]

I'm personally fine with merging this despite the lack of tests, C# for Breathe is generally speaking not tested on this side currently, it was introduced with relatively simple PR #550 and so far afaik mainly sprinkles some if dom == 'cs' in specific areas, meaning the code will never run if it's not C#, so it can never break C/C++ in the first place.

In fact, C# support for Breathe is so niche it's not even mentioned in docs, it is only mentioned in changelog entries. I think it's safe to say that @rogerbarton is de-facto C# maintainer for Breathe at this point, considering he maintains the domain as well.

$ grep -rIin -e '[cC]#' -e '[cC][- _]sharp'
CHANGELOG.rst:42:  - Do not add inline modifier for C#. `#668 <https://github.com/michaeljones/breathe/pull/668>`__
CHANGELOG.rst:55:  - Add missing getter and setter for C#. `#661 <https://github.com/michaeljones/breathe/pull/661>`__
CHANGELOG.rst:125:  - Add support for C# using sphinx-csharp. `#550 <https://github.com/michaeljones/breathe/pull/550>`__

@michaeljones @jakobandersen Thoughts on creating a follow-up issue for C# doc/tests and getting this merged for the fix for now?

[jakobandersen]

@michaeljones @jakobandersen Thoughts on creating a follow-up issue for C# doc/tests and getting this merged for the fix for now?

As you mention, the C# support is basically undocumented so I'm fine with merging as is and deferring the tests and docs for a later time.

[rogerbarton]

I tried adding a C# test today, but I think this will just complicate the testing situation without much gain.
For example, you would need to have sphinx-csharp installed or else building the docs would fail.

I agree that the C# integration is quite harmless with mostly if dom == 'cs' checks. So I would be for merging this.
When more people are using the C# domain we can look at testing...
I should also write documentation first on how to use it.

[michaeljones]

Thank you for breaking it down @vermeeren. Very useful for some one as out of touch as I am. I agree with the reasoning. Merge away :)

[michaeljones]

I guess I might as well do it :)