Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verbatim comments translation for C# #1695

Closed
wants to merge 3 commits into from

Conversation

wkalinin
Copy link
Contributor

@wkalinin wkalinin commented Jan 4, 2020

Added literal doxygen comments translation for C#, which is enough for our needs. Hope other C# users would find that useful.

Copy link
Member

@vadz vadz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I don't know how well is it going to work in practice, but it's definitely better than nothing. Out of curiosity, do you know what would the full implementation do, i.e. what should be translated to what?

It would be nice to enable at least the basic Doxygen tests (not using translation) for C#, having them pass would help to ensure that this doesn't get broken in the future.

@@ -155,6 +161,7 @@ class CSHARP:public Language {
director_method_types(NULL),
director_connect_parms(NULL),
destructor_call(NULL),
structuralComments(NULL),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This results in

Source/Modules/csharp.cxx:164:7: warning: field 'structuralComments' will be initialized after field 'output_file' [-Wreorder]

which is harmless here but still would be better to fix.

@@ -46,6 +47,8 @@ class CSHARP:public Language {
bool global_variable_flag; // Flag for when wrapping a global variable
bool old_variable_names; // Flag for old style variable names in the intermediary class
bool generate_property_declaration_flag; // Flag for generating properties
bool doxygen; //flag enabling comment processing
bool comment_creation_chatter; //flag for getting information about where comments were created
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this supposed to be left in? Because there doesn't seem to be any way to set it, so it doesn't look useful in its current form.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe set from the debugger (just like in Java module)

@@ -1434,6 +1514,22 @@ class CSHARP:public Language {
return SWIG_OK;
}

/* -----------------------------------------------------------------------
* doxygenComment()
* Simply translates the doxygen comment and places it into the appropriate
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if "translates" here is really correct, as it doesn't seem to be doing any translation, does it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends on the translator, just default being verbatim copy. I mean, technically C# module knows that the translator is doing nothing only in module constructor (that may change if someone will introduce an alternative).

@@ -1456,6 +1552,15 @@ class CSHARP:public Language {
Swig_save("constantWrapper", n, "value", NIL);
Swig_save("constantWrapper", n, "tmap:ctype:out", "tmap:imtype:out", "tmap:cstype:out", "tmap:out:null", "tmap:imtype:outattributes", "tmap:cstype:outattributes", NIL);

//translate and write comment if flagged
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, could you please explain what do you mean by "if flagged" here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blind copy from the Java module, probably means "if enabled", removed now.

@vadz vadz added C# Doxygen Related to -doxygen command line option labels Jan 5, 2020
@wkalinin
Copy link
Contributor Author

wkalinin commented Jan 6, 2020

This is actually very old code, added when doxygen was just released in it's own branch. I'll fix the warning and remove the useless comments
I have no idea what comments may be translated to, our documentation tool supports Doxygen in C# just fine, so I needed to only copy the comments.
There are quite a bit of doxygen tests in Java test suite, what test would you recommend as essential?
(Sorry for the spam, did not relogin into real account)

@ojwb
Copy link
Member

ojwb commented Mar 10, 2022

I think it would be best to have all the doxygen testcases for each supported language.

While there are quite a few, it looks like most of the hard work for a new language is implementing something equivalent to CommentParser which gets hold of the output doxygen comments, normalises it, and compares with the expected version.

The rest is an essentially mechanical conversion of the testcases, and the bulk of that could be scripted.

@emmenlau
Copy link
Contributor

emmenlau commented Sep 9, 2022

Doxygen support for C# would be great to have, so just kindly bumping this up again...

@wsfulton
Copy link
Member

wsfulton commented Sep 9, 2022

Converting and adding the Java doxygen tests for this C# doxygen support is what is required for this to be included into SWIG. The testcases that need converting from Java to C# are:

$ ls Examples/test-suite/java/doxygen_*runme.java
Examples/test-suite/java/doxygen_alias_runme.java
Examples/test-suite/java/doxygen_basic_notranslate_runme.java
Examples/test-suite/java/doxygen_basic_translate_runme.java
Examples/test-suite/java/doxygen_basic_translate_style2_runme.java
Examples/test-suite/java/doxygen_basic_translate_style3_runme.java
Examples/test-suite/java/doxygen_code_blocks_runme.java
Examples/test-suite/java/doxygen_ignore_runme.java
Examples/test-suite/java/doxygen_misc_constructs_runme.java
Examples/test-suite/java/doxygen_nested_class_runme.java
Examples/test-suite/java/doxygen_parsing_enums_proper_runme.java
Examples/test-suite/java/doxygen_parsing_enums_simple_runme.java
Examples/test-suite/java/doxygen_parsing_enums_typesafe_runme.java
Examples/test-suite/java/doxygen_parsing_enums_typeunsafe_runme.java
Examples/test-suite/java/doxygen_parsing_runme.java
Examples/test-suite/java/doxygen_translate_all_tags_runme.java
Examples/test-suite/java/doxygen_translate_links_runme.java
Examples/test-suite/java/doxygen_translate_runme.java

Converting Java code to C# code is normally quite easy.

@emmenlau
Copy link
Contributor

@wkalinin you have already done a lot for C# doxygen support! Would you be willing to push this further? I don't feel confident that I'm ready to start here, but doxygen support would be really nice to have in master...

@emmenlau
Copy link
Contributor

Dear @wkalinin, based on your nice work in this PR, I have continued with C# documentation support in #2421. My PR adds a few extensions on top of yours. I left your author name etc intact, and hope your appreciate my continued work.

Please let me know if this is in your best interest? Then we can possibly close one of the two merge requests and continue the collaborative effort in the remaining merge request. Your input and help is appreciated!

@ojwb
Copy link
Member

ojwb commented May 8, 2023

Closing in favour of #2421.

@ojwb ojwb closed this May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C# Doxygen Related to -doxygen command line option
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants