jaeandersson / swig

SWIG is a software development tool that connects programs written in C and C++ with a variety of high-level programming languages.
http://www.swig.org
Other
23 stars 19 forks source link

Fix wrapping of %define and #define expressions containing chars #93

Open nunoguedelha opened 6 years ago

nunoguedelha commented 6 years ago

Hi @jaeandersson , as per our short discussion, here is the PR fixing the wrapping of global defines and enums having char type elements, or being expanded with char type inputs. Before the fix, the chars would lose the quotes and become variables.

Please refer to https://github.com/robotology-dependencies/swig/pull/1 for more details.

nunoguedelha commented 6 years ago

@jaeandersson , if I may, I suggest you do a fast-forward merge in case you accept the PR. This way we avoid diverging our matlab branches. Feel free to contact us if you have questions on the further fixes we have made on top of this one on our matlab branch: https://github.com/robotology-dependencies/swig/ pull/2 https://github.com/robotology-dependencies/swig/commit/7e61db6b5dc841027439081aed3bf8c36c9e1920

nunoguedelha commented 6 years ago

CC @traversaro

wsfulton commented 6 years ago

Who and where from is the original source of these changes? Shouldn't Go and C# pull requests be submitted to SWIG proper?

nunoguedelha commented 6 years ago

This PR merges the fix https://github.com/swig/swig/pull/781, please refer to https://github.com/robotology-dependencies/swig/pull/1 for more details.

nunoguedelha commented 6 years ago

Hi @wsfulton , the original fix https://github.com/swig/swig/pull/781 is already in the master branch of the original repo https://github.com/swig/swig, but not on the matlab branch. Please Let us know how we should proceed. It might be an opportunity to check if we can PR the other changes from jaeandersson:swig:matlab branch into swig:swig:matlab.

wsfulton commented 6 years ago

That's okay, it wasn't clear to me when I first saw this that they came from swig/swig originally.

jaeandersson commented 6 years ago

Thank you for your contribution. I'll check in the next couple of days. The Matlab branch really needs to get the last push for merging with swig proper. I'll make it a priority. My last couple of months have been pretty crazy, but it's starting to calm down now.

KrisThielemans commented 2 years ago

Can this be closed please? It's obsolete after merging swig/master in #96.