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

Fix for #163 : Improved import detection for REST interface generators #164

Merged
merged 3 commits into from Jan 21, 2013

Conversation

Projects
None yet
2 participants
@mihails-strasuns
Contributor

mihails-strasuns commented Jan 21, 2013

Follow-up for #163
I have no idea how to forge external modules for unit-testing generateModuleImports so tested only on my code. Please review carefully.

I wonder if some more generic type/symbol processing primitives can be designed (and included in phobos) instead of stockpiling similar utility functions.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 21, 2013

Member

Looks good as far as I can see. I would only change the alias syntax back to the old alias xyz ident; syntax to keep the code 2.060 compatible until 2.062 is released (it was great to see the new syntax added, though - much cleaner and consistent with the rest of D).

Regarding Phobos inclusion.. maybe something like UserTypeTuple!T with additional support for functions/delegates would be general enough? Sounds like it might be useful for some other things.

Member

s-ludwig commented Jan 21, 2013

Looks good as far as I can see. I would only change the alias syntax back to the old alias xyz ident; syntax to keep the code 2.060 compatible until 2.062 is released (it was great to see the new syntax added, though - much cleaner and consistent with the rest of D).

Regarding Phobos inclusion.. maybe something like UserTypeTuple!T with additional support for functions/delegates would be general enough? Sounds like it might be useful for some other things.

@mihails-strasuns

This comment has been minimized.

Show comment
Hide comment
@mihails-strasuns

mihails-strasuns Jan 21, 2013

Contributor

Sure. Does 2 last DMD version compatibility is general vibe.d policy now?

What should UserTypeTuple do?
I am eager to write something while wisdom provided by Kenji about D type system is still fresh in my mind :) I.e. I have noticed that full and proper implementation of fullyQualifiedName ( which has been merged recently ) almost completely duplicates most code in vibe.http.rest which currently does function signature copying. And I have started thinking about more generic code reuse approach for those tasks. I can't just add phobos pull request for every special case encountered while exploring vibe - I would have been first one to reject them :)

That was a mostly rhetorical question though, I do not think an obvious approach is possible here.

Contributor

mihails-strasuns commented Jan 21, 2013

Sure. Does 2 last DMD version compatibility is general vibe.d policy now?

What should UserTypeTuple do?
I am eager to write something while wisdom provided by Kenji about D type system is still fresh in my mind :) I.e. I have noticed that full and proper implementation of fullyQualifiedName ( which has been merged recently ) almost completely duplicates most code in vibe.http.rest which currently does function signature copying. And I have started thinking about more generic code reuse approach for those tasks. I can't just add phobos pull request for every special case encountered while exploring vibe - I would have been first one to reject them :)

That was a mostly rhetorical question though, I do not think an obvious approach is possible here.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 21, 2013

Member

Yeah I wanted to at least always support the previous DMD version to make the transition less painful. People (partially including myself on different machines ;) ) just have too different upgrade policies.

UserTypeTuple would be the same as getSymbols, just handling all kinds of types and returns a tuple with every user defined type that occurs in that type. But I agree that this gets a bit more special than for example BaseTypeTuple or MemberFunctionTuple... so maybe it gets too specific for std.traits.

Member

s-ludwig commented Jan 21, 2013

Yeah I wanted to at least always support the previous DMD version to make the transition less painful. People (partially including myself on different machines ;) ) just have too different upgrade policies.

UserTypeTuple would be the same as getSymbols, just handling all kinds of types and returns a tuple with every user defined type that occurs in that type. But I agree that this gets a bit more special than for example BaseTypeTuple or MemberFunctionTuple... so maybe it gets too specific for std.traits.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jan 21, 2013

Member

Thanks for the alias change. I'll merge and test on 2.060.

Member

s-ludwig commented Jan 21, 2013

Thanks for the alias change. I'll merge and test on 2.060.

s-ludwig added a commit that referenced this pull request Jan 21, 2013

Merge pull request #164 from Dicebot/generate_imports
Fix for #163 : Improved import detection for REST interface generators

@s-ludwig s-ludwig merged commit 4c8561d into vibe-d:master Jan 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment