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

Remove template arguments from the class name in the member function #191

Merged
merged 2 commits into from
May 29, 2015

Conversation

vitaut
Copy link
Contributor

@vitaut vitaut commented May 26, 2015

The proposed PR removes template arguments from the class name in the member function. Otherwise links to nested classes (in parameter or return types) defined in the same template are not generated correctly, because the latter don't include template arguments.

For example, void C<T>::f(D) is converted to void C::f(D) in

/** A class */
template <typename T>
class C {
 public:
  /** An inner class. */
  class D {};

  /** A method. */
  void f(D);
};

Fixes #179.

@michaeljones, if you merged #189 first, I could also add a unit test for this.

@michaeljones
Copy link
Collaborator

Thanks for the prompting, I've just merged and pushed that other branch.

I haven't had a chance to properly look at your code. I'm sure it is great but I'd agree with you that it is the sort of thing that goes well with a test or two to double check.

Thanks for picking up on the issue and fixing it as always. Sounds like a good choice to me.

Otherwise links to nested classes (in parameter or return types) defined
in the same template are not generated correctly, because the latter
don't include template arguments. Fixes #179.
@vitaut
Copy link
Contributor Author

vitaut commented May 27, 2015

Thanks for merging the unit testing branch. I've moved the code that removes template arguments to a separate function and added a unit test for it.

And add a couple of comments to help me remember how it works.
@michaeljones
Copy link
Collaborator

It looks great. Thanks for catching it and fixing it. I've just adjusted the indentation in the test code and added a comment or two to help me out.

Much appreciated as always. Merge if you're happy with the changes.

@vitaut vitaut merged commit aaf28c5 into master May 29, 2015
@vitaut
Copy link
Contributor Author

vitaut commented May 29, 2015

Thanks for the review and code improvements. Sorry about about the mess with indents, I got used to 2-char indents and sometimes forget to adapt to the surrounding code =).

@michaeljones
Copy link
Collaborator

No trouble at all. I sometimes flirt with 2-space indentation and understand how annoying it is to keep per-project configurations. An understandable situation!

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.

Handling of template members
2 participants