SWI-Prolog / packages-pengines

Pengines: Prolog engines
12 stars 13 forks source link

Format option bug #15

Closed torbjornlager closed 10 years ago

torbjornlager commented 10 years ago

An example such as this shouldn't work:

:- use_module(library(pengines)).

test :-
    pengine_create([
        format(json),
        ask(member(_X, [a,b,c]))
    ]),
    pengine_event_loop(handler, []).

handler(success(ID, X, true)) :-
    writeln(X),
    pengine_next(ID, []).
handler(success(_ID, X, false)) :-
    writeln(X).

but it does. In prints

?- test.
[member(a,[a,b,c])]
[member(b,[a,b,c])]
[member(c,[a,b,c])]
true.

despite the fact that format(json) is passed to pengine_create/1.

What I would expect here is that the argument to handler/1 be JSON. The format option seems to have no effect at all in this example.

torbjornlager commented 10 years ago

A solution may be to disallow the format option in a case like this. Here's a use case trying to make use of it though, where the intention is to forwards the answer to another channel (in this case the chat server), where JSON might be expected:

:- module(pengine_bot,
      [     pengine_bot_enter/2
      ]).

:- use_module(library(http/websocket)).
:- use_module(library(pengines)).
:- use_module(library(option)).

% pengine_enter('http://localhost:3050', [alias(foo), destroy(false)]).

pengine_bot_enter(URL, Options) :-
    atomic_list_concat([URL, '/chat'], WSURL),
    http_open_websocket(WSURL, WebSocket, []),
    pengine_create(Options),
    pengine_event_loop(send_and_receive(WebSocket, Options), []).    

send_and_receive(WebSocket, Options, PengineEvent) :-
    option(format(Format), Options, prolog),
    (   Format == prolog
    ->  ws_send(WebSocket, prolog(PengineEvent))
    ;   Format == json
    ->  ws_send(WebSocket, json(PengineEvent))
    ),
    receive(WebSocket).

% has to be prolog
receive(WebSocket) :-     
    (   ws_receive(WebSocket, Message, [
            format(prolog), 
            syntax_errors(quiet)
        ]),
        interpret(Message.data)
    ->  true
    ;   receive(WebSocket)
    ).

interpret(ID < ask(Goal)) :-
    pengine_ask(ID, Goal, []).
interpret(ID < ask(Goal, Options)) :-
    pengine_ask(ID, Goal, Options).
interpret(ID < next) :-
    pengine_next(ID, []).
interpret(ID < stop) :-
    pengine_stop(ID, []).
interpret(ID < abort) :-
    pengine_abort(ID, []).
interpret(ID < destroy) :-
    pengine_destroy(ID, []).  
torbjornlager commented 10 years ago

I'm strongly leaning towards removing the format option from pengine_create/1. Making it really work would require us to rewrite (e.g.) pengine_event_loop/2 and even pengine_rpc/3 in order to handle JSON and all the other formats. The use case I gave above does in no way warrant such a move.

Removing the option requires no change of code, only the removal of the following lines from the documentation of pengine_create/1:

    * format(+Format)
      Determines the format of event responses. Format is an atom.
      The default format is =prolog=.  In addition, =json= is supported
      and new formats can be added by defining event_to_json/3.  See
      library(pengines_io).

Instead, a revised version of that comment should be added to the JavaScript documentation, in the description of the format option of new Pengine({...}):

      format:+Format
      Determines the format of event responses. Format is a string.
      The default format is =json=.  In addition, =json-s= and =json-html= are supported
      and new formats can be added by defining event_to_json/3.  See
      library(pengines_io).

Do you agree?

JanWielemaker commented 10 years ago

Makes sense. pengine_create/3 creates a Prolog pengine. I can't really see a sensible reason to make that talk anything but Prolog. Especially for local pengines.