Skip to content

Fix extending class in subpackage.#92

Merged
jaeandersson merged 1 commit into
jaeandersson:matlabfrom
yuriy-kozlov:matlab
Jan 24, 2018
Merged

Fix extending class in subpackage.#92
jaeandersson merged 1 commit into
jaeandersson:matlabfrom
yuriy-kozlov:matlab

Conversation

@yuriy-kozlov
Copy link
Copy Markdown

Use base class package name rather than module name because
MATLAB requires it to be fully qualified.

Use base class package name rather than module name because
 MATLAB requires it to be fully qualified.
@yuriy-kozlov
Copy link
Copy Markdown
Author

This fixes #55 or at least part of it.

@jaeandersson jaeandersson merged commit 59e0f55 into jaeandersson:matlab Jan 24, 2018
@jaeandersson
Copy link
Copy Markdown
Owner

Thank you for your contribution!

@nunoguedelha
Copy link
Copy Markdown

This PR seems to break previous configurations for generating bindings, typically in simple cases where we generate all bindings for a single module, into a single package folder, both with same name. Here is an example of issue: robotology-dependencies#3.
@yuriy-kozlov @jaeandersson it would be nice to have some more details or a clear explanation of the impacts on the module definition (in the module file <module>.i) or the generated tree structure.

@nunoguedelha
Copy link
Copy Markdown

CC @traversaro

@yuriy-kozlov
Copy link
Copy Markdown
Author

@nunoguedelha do you specify the package= option in %module? I have it everywhere and I think missing it is triggering this bug. It should be optional, of course, so I can work on a fix.

@nunoguedelha
Copy link
Copy Markdown

@yuriy-kozlov thank you, I'll try it ASAP and let you know. Of course, if you can make it optional it would be great, because in our case I think the option would be over-killing, since we always have a single module per package.

@jaeandersson
Copy link
Copy Markdown
Owner

So, should this commit be reverted then?

@nunoguedelha
Copy link
Copy Markdown

Personally, I realize I won't have time to run further tests following @yuriy-kozlov recommendations. I was hopping we could get a fix from him for making the feature optional, and so backward compatible with old configurations. Otherwise I suggest to revert the commit.

Alzathar added a commit to Alzathar/swig that referenced this pull request Dec 28, 2019
 - Based on the main Swig project
 - Using the content of the PR swig#159
 - But the PR jaeandersson#92 was removed
   as its breaks class inheritance
KrisThielemans added a commit to KrisThielemans/swig that referenced this pull request Jul 30, 2021
This reverts commit 2a0c344, i.e.
jaeandersson#92 due to problems mentioned
there.
@KrisThielemans KrisThielemans mentioned this pull request Jul 30, 2021
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants