search

SWI-Prolog / packages-pengines

Pengines: Prolog engines
12 stars 13 forks source link

Fixed blob to HTML conversion and added rendering hook. #18

Closed samer-- closed 9 years ago

samer-- commented 9 years ago

The problem was that term_html:term//3 was producing an assertion failure on line 88 when asked to render a term containing blobs, beause primitive/2 was not catching the blobs.

The changes are self-explanatory, I think -- the only question is which module to declare the blob_rendering//3 hook. I put it in pengines_io because there was already a hook there, but perhaps it should be in term_html.pl so as not to depend on pengines_io.pl. In fact, this version might not work correctly because pengines_io is not imported into term_html, so term_html does not know that pengines_io:blob_rendering//3 is multifile. Thus, if external code declares clauses of blob_rendering//3, they might clash or overwrite each other -- my understanding of multifile predicates leaves me unsure, so please feel free to re-organise the hook predicates as you see fit.

JanWielemaker commented 9 years ago

Included after some patches. Notably, I moved the hook to term_html.pl. This library can be used independently from pengines_io. Also ensures that the hook is not called for normal atoms and []. Thanks!

Typically, do not send pull requests from the master branch. Make sure master remains in sync with the central repo. To propose a fix, make a temporary branch from an up-to-date master and issue a pull request from there. Next, you can simply stay in sync by deleting the temporary branch after it has been merged.

samer-- commented 9 years ago

Thanks, yes, I just made some fixes to my copy too, to prevent blob rendering for normal atoms which are apparently blobs of type text. Also, I excluded blob type of 'reserved_symbol' rather than just []. Are there any other reserved symbols?

Another thing that just occurred to me is that Pengine term rendering is completely independent of the the portray/1 subsystem. Was that a conscious decision? I can see how they are somewhat incompatible (since portray just produces plain text with no internal structure), so perhaps there is no point pursuing that line of thought..

Samer

On 19 Dec 2014, at 15:56, Jan Wielemaker notifications@github.com wrote:

Included after some patches. Notably, I moved the hook to term_html.pl. This library can be used independently from pengines_io. Also ensures that the hook is not called for normal atoms and []. Thanks!

Typically, do not send pull requests from the master branch. Make sure master remains in sync with the central repo. To propose a fix, make a temporary branch from an up-to-date master and issue a pull request from there. Next, you can simply stay in sync by deleting the temporary branch after it has been merged.

— Reply to this email directly or view it on GitHub.

JanWielemaker commented 9 years ago

As yet, [] is the only reserved symbol. As I think of it, the best default is probably to call write/1 on them.

Yes, the infrastructure is deliberately distinct from portray. I want proper HTML rendering of terms.