ridgeworks / clpBNR

CLP(BNR) module for SWI-Prolog
MIT License
38 stars 5 forks source link

Two more issues #24

Closed JanWielemaker closed 4 months ago

JanWielemaker commented 4 months ago

First is an invalid version. This caused a crash in swish. Also the prolog:rational caused an error, but that is fixed at the Prolog end.

The other renames the notebook file. Linking from the example pages to stored pages does not work and that may be better as the one is from swish and the other from a particular installation. It also didn't like a file name with () and spaces.

So with this PR, you see what is running on SWISH. Unfortunately this means that updating the tutorial requires updating SWISH. Shouldn't be too often, I assume.

ridgeworks commented 4 months ago

Can you clarify the notebook file behaviour, i.e., what happens when I edit its content? It appears that any direct link sees the changes but the link from the tutorial notebook will link to whatever was there when SWISH was built. Any changes will not be reflected until SWISH is rebuilt. (??)

I have copied the content to a new notebook clpBNR_quickstart.swinb and will use that from now on. Any way to delete the old file?

JanWielemaker commented 4 months ago

SWISH uses the swish/clpBNR_quickstart.swinb from the pack. So, the only way to update is to update SWISH (well, the clpBNR pack attached to it). Getting it any easier is a bit too much fuzz :cry:

ridgeworks commented 4 months ago

OK, so I totally misunderstood. There are in fact two copies of clpBNR_quickstart.swinb on SWISH. One is copied from this repo whenever SWISH is rebuilt and one is the attached my account. It'll be up to me to keep the two in sync by keeping the repo "up to date".

ridgeworks commented 4 months ago

I see that this and clpBNR 0.11.6 are now available on SWISH; thanks.

A main focus of 0.11.6 was pldoc for help/1 and I've noticed an issue with man_page//2. If a predicate is documented in the manual at least once, no other pldoc for that predicate is produced. In this case, {}/1 is implemented in modules clpqr and semweb/rdfill and, now, clpBNR. Now:

  1. Given ?- help(clpBNR)., all the doc is displayed except for {}/1.
  2. Given ?- help({}). or ?- help({}/1)., help for clpqr:{}/1 and semweb/rdfill:{}/1 are displayed but not clpBNR:{}/1. Further the availability header for the clpqr version of the pldoc indicates it comes from clpBNR.
  3. Given help(clpBNR:{}/1). or help(clpBNR:{}/1)., it says there is no No help for clpBNR:{}/1. Use ?- apropos(query). to search for candidates.

The problem seems to come from man_page//2. The second clause checks for a one or more manual entries for the predicate and upon success cuts the alternative third clause which would have collected the pldoc for predicates available but not documented in the manual.

I'm happy to work on a fix for this, but I wanted to get input on overall strategy and possible corner cases to watch out for.

JanWielemaker commented 4 months ago

For me, it didn't display any help after

swipl prolog/clpBNR.pl
?- help(clpBNR).

Now it does. That was an issue with exported predicates that are defined in included files, were we must reload the main file to process the comments rather than the included file. It also works for several predicates, but not for all. E.g.,

?- help(interval).

gives no results either. I suspect that included files are involved here too. No real clue how. Just needs more debugging ...

ridgeworks commented 4 months ago

So far all testing done on a local version of SWIP (i.e., not SWISH).

I think at the end of the day, the problem is that there doesn't seem to be any way of qualifying manual doc content with its module/source. So, for example, while I can fix the problem so that ?- help({}). displays help for all instances of {} including clpBNR, I can't selectively display content, e.g.,:

?- help(clpBNR:{}).
Warning: No help for clpBNR:{}.
Warning: Use ?- apropos(query). to search for candidates.
true.

?- help(clpqr:{}).
Warning: No help for clpqr:{}.
Warning: Use ?- apropos(query). to search for candidates.
true.

?- help(clpqr:{}/1).
Warning: No help for clpqr:{}/1.
Warning: Use ?- apropos(query). to search for candidates.
true.

Also clpBNR:{} doesn't appear in the clpBNR module level help because there is a manual entry for clpqr:{} (see prolog_manual_index:current_predicate_help/1) and the manual object lookup can't distinguish between the two.

In addition, the synopsis info is incorrect because the manual entry for clpqr:{}/1 ends up defaulting to clpBNR if it's loaded:

?- help({}/1).
                                                  Availability: :- use_module(library(clpBNR)).

{}(+Constraints)
    Adds the constraints given by Constraints to the constraint store.

... etc. ...

So I think the root cause of these issues is that the pldoc data doesn't seem to preserve its module context, which goes pretty deep into the whole pldoc system. (And I don't think any of this has to do with included files.)

I'll keep digging, but of you have any insights, please share.

JanWielemaker commented 4 months ago

So I think the root cause of these issues is that the pldoc data doesn't seem to preserve its module context, which goes pretty deep into the whole pldoc system. (And I don't think any of this has to do with included files.)

Pushed some enhancements that makes ?- help(clpBNR:{}). work. The PlDoc rendering can deal with qualified predicates. The main issue is that we do not know the module for most predicates in the manual. That was not an issue when the documentation system was built as there was no ambiguity. It means that the availability statement is generated with some dubious assumptions though :cry:

ridgeworks commented 4 months ago

I was looking in the same area. I had to make an additional change so it would work in for my various ad hoc tests:

The help_object/4 clause after the two you added needs to add the Module to the found Obj, as in:

help_object(Name/Arity, _How, Module:Name/Arity, _ID) :-
    atom(Name),
    current_predicate_help(Module:Name/Arity).

This is necessary so that ?- help({}/1). will produce the loaded predicate entry, in this case for clpBNR.

I agree the "Availability" statement is suspect and a fix may not be simple.

JanWielemaker commented 4 months ago

Agree. Finally also found why part of the comments are lost after including a file. Seems we are now in a reasonable state ...