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

Support simple extension functions written in JavaScript #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

gagern
Copy link
Collaborator

@gagern gagern commented Jan 30, 2015

As suggested in #2, here is my stab at getting extension functions implemented. If you don't feel like merging this until more types are supported, let me know. But I'm not sure I'll be able to provide support for additional types any time soon, if at all.

For asynchroneous calls, the callbacks are performed on the main event loop
while the worker thread is blocked on a mutex.  At the moment, only boolean,
numeric and string values can be passed between the XSLT processor and the
JavaScript function.  Other types would be highly desirable, but might
require a lot more work.
This ensures that the main event loop will terminate without a call to
process.exit.  And it satisfies the requirement from the manual that the
async must be closed before being deallocated.  Closing on the v8 thread
avoids an abort when v8 itself shuts down, details unknown.
@gagern
Copy link
Collaborator Author

gagern commented Jan 30, 2015

OK, apparently CI is unhappy about how I set the flag for C++11 support. This question on Stack Overflow might help to get things running on Mac, but doesn't seem to have anything to offer for here. Any better suggestions?

@albanm
Copy link
Owner

albanm commented Feb 1, 2015

I didn't read your whole commits, but I guess c++11 is required for lambda functions ? During my tests for this functionality, it seemed necessary to use lambda functions to wap V8 functions into extension functions compatible with libxslt. I spoke about functors in the comment of the commit by mistake I actually meant lambda function.

I saw this promising post about building node extensions with C++11, and that is the limit of my knowledge on this question :(

I have absolutely no problem accepting the PR with only simple types support. Only the build issue has to be fixed and the documentation completed. Beware that the README.md should not be edited directly, instead you should edit README.hbs then run 'npm run-script doc'.

The only thing bothering me is that this is a significant jump in code complexity for this project. Honestly, I am not a competent C++ dev, and I don't feel like maintaining another developer's code in this technology. Anyway I lack the time to do so. Would you agree to be registered as contributor to the project ?

@gagern
Copy link
Collaborator Author

gagern commented Feb 1, 2015

I don't need lambda functions. I'm mostly using C++11 because I like unique_ptr. I thought up front that we'd need C++11's mutexes as well, but now I'm using libuv's. I guess that if I invest a bit more care into proper manual memory management, I could turn this all into C++03 as well.

I guess the Travis problem lies in the fact that it's using GCC 4.6.3 which doesn't know -std=c++11 yet, only -std=c++0x. I'll have to check whether all relevant features are available with that. Do you have any other rules about which compilers you officially support? Is there some list what NPM might or might not throw at your code? Clearly C++11 is the way forward, so I'm reluctant to uglify my code just to clean it up in half a year. On the other hand, it's still fairly new, and there are a lot of different compilers out there, many of which won't know about it, much less the flag I'd use to specify its use.

I'd gladly come aboard to support extension function in the future, but since I have near to no knowledge about V8, NAN and the whole NPM development and deployment workflow, I'd be looking for guidance in those areas. And I won't make any binding promises. I'f I'm too busy to address some issue, then that's that. I don't mind friendly reminders if I forget something, though.

@gagern
Copy link
Collaborator Author

gagern commented Feb 2, 2015

I changed the syntax to C++03 but there appears to be some race condition, probably related to how I close the async handle. Not sure how to do this correctly. I saw this error before when I closed the handle in the destructor.

The core idea is that we allocate the context before we apply the
stylesheet.  That way we can store task-specific stuff in that context, like
the information whether we are single- or multi-threaded.  We might also be
storing per-context functions there.
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.

2 participants