Hello,
To add tests put something suitable into the inria suite in
test/inriasuite.
They can be run with:
gnu.prolog.test.GoalRunner --once inriasuite.pl run_all_tests
Predicate_setup_call_catcher_cleanup wants a copyright header and a
package. It also needs to be wired in via an entry in a .pro in
gnu.prolog.vm.buildins. I would prefer it in a patch rather than as the
single file as a patch has metadata about where it should go.
Which version of gnuprologjava did you write
Predicate_setup_call_catcher_cleanup? 0.2.6? It isn't current head
anyway - which doesn't matter I can do a 0.2.7 release with this in and
port the code to the current newmaster.
In terms of imports I prefer to avoid using foo.bar.* as when later I do
Ctrl-Shift-o in eclipse they will get expanded.
When using stuff from args[] if you are only using a particular index
once then it is ok to not assign it to a variable with a meaningful name
but when using a particular index more than once it is much more
readable for them to be named by assigning to a variable at the
beginning. Probably also good to check that that args[] is the right
length (though with correct wiring up it should always get the right
number of arguments).
When ignoring Exceptions a comment is required explaining why this is
sensible. The correct thing to do in this case would probably be to log
this at info or warning but we don't have a logging framework a TODO
would be helpful for when I add that in later.
BacktrackInfoWithCleanup needs its copyright header updating as it
doesn't have you on it and if should only have Constantine on it if you
are reusing code that Constantine wrote.
Similarly a TODO about logging the exception would be helpful.
With those addressed and tests to prove it works and to prevent me
breaking it then that should be fine.
Thank you,
Daniel
On Thu, 2011-12-15 at 11:18 +1300, Matt Lilley wrote:
Hello,
I've attached a patch that does the trick. I don't think it's too
inefficient, though it does make popping frames a bit slower (I don't
think there's any way you can avoid that if the task at hand is to add
complexity to that very process). Also attached is an implementation of
Predicate_setup_call_catcher_cleanup, the more general case of
setup_call_cleanup/3. I'm not entirely sure how your testing framework
works, but I can provide a prolog file as well if that would help.
I'm happy to rework this if you have suggestions on how it could be
better integrated; I'm obviously very new to your project!
Thanks again,
Matt
Hello,
If you want to submit a patch to add that functionality then that would
be great, I have no problem with adding non-ISO features as long as ISO
features don't get broken and as long as it can be implemented cleanly.
Unfortunately I don't have time to work out how to write such a patch
myself at the moment but I do have time to review it. If you could add
some test cases to prove it works that would be great.
If implementing it cleanly is not possible then we can look at what
could be changed to either make that possible or to allow a local hack.
If you have any questions then please ask.
Best regards,
Daniel
0001-Support-executing-cleanup-code-when-backtracking.patch