search

Interlisp / medley

The main repo for the Medley Interlisp project. Wiki, Issues are here. Other repositories include maiko (the VM implementation) and Interlisp.github.io (web site sources)
https://Interlisp.org
MIT License
378 stars 19 forks source link

Class not found error when inputting class name in CLOS browser #1393

Open pamoroso opened 1 year ago

pamoroso commented 1 year ago

Describe the bug When entering any class name as input to the CLOS browser the tool breaks with the error No class named: CLASSNAME , where CLASSNAME is the input, regardless of whether the class exists or not.

To Reproduce Steps to reproduce the behavior:

  1. sign into your Interlisp Online account
  2. under Initial Exec, select Common Lisp
  3. click Run Medley
  4. on the desktop background, right-click and select CLOS Browse Class
  5. in the prompt area of the CLOS browser, type STANDARD-OBJECT and press ENTER

Expected behavior A class hierarchy diagram is displayed without errors.

Screenshots The break window:

class-not-found-error

The full desktop:

class-not-found-error-desktop

Context (please complete the following information):

Additional context The error occurs whenever entering a class name as input to any command that takes such an input like Add root. Even when the class doesn't exist it would be more user-friendly to just display a warning in the prompt area.

nbriggs commented 1 year ago

CLOS classes are all (mostly?) in the CLOS package, so entering, for example, CLOS::STANDARD-OBJECT to the class browser should get you what you expect.

pamoroso commented 1 year ago

Wouldn't it be better to display a warning without breaking when the name isn't recognized or the class doesn't exist?

nbriggs commented 1 year ago

Yes, but -- clos:find-class does (approximately) (cl:error "Class not found: ~s" class-name) and I don't think the higher level calls (in particular the class-browser as it is called from the background menu) are catching the error - they're probably expecting the message to get printed in the promptwindow. I haven't read the CLOS documentation to see if it mentions the expected behavior of the class browser in the case the class doesn't exist.

pamoroso commented 1 year ago

I've just checked the documentation of Medley's CLOS browser and it says nothing concerning the case the class doesn't exist.

pamoroso commented 1 year ago

find-class takes an errorp parameter to control what to do in case the class doesn't exist.

pamoroso commented 3 months ago

The issue seems localized to function clos-browser:browse-class of file clos/NEW-CLOS-BROWSER.

The expression that calculates the value of root-classes in the let* bindings passes find-class to mapcar, which results in an error being singaled if the class is not found as the errorp argument of find-class defaults to true.

A possible fix is to wrap find-class in a lambda that passes nil as the value of errorp. Any nils find-class returns when no class is found should be removed from the value of lambda calls. In other words, code like this:

(remove nil (mapcar #'(lambda (c) (find-class c nil)) class-name-or-list))

Currently root-classes is a list of one non nil element, if the class whose name the user inputs is found, or the classes in a package the user inputs. To complete the fix the function should check whether root-classes is nil and if so call web:prompt-print to warn no such class is found. Otherwise the body of the function can be ran as usual.

I believe the expressions that calculate the values of the bindings that follow root-classes in let* require no changes.

MattHeffron commented 2 months ago

@pamoroso Actually, it is a bit trickier. :wink: In browse-class (I'm leaving off the package name) a (remove nil ...) should be wrapped around the (IF (LISTP ...) (MAPCAR ...) (CONS ...)) since in either branch the find-class wrapper could return NIL. That wrapper should be defined in an (flet ...) surrounding the LET*, since it is used in both branches.

Alternatively: before the current (LET* there could be a different find-class wrapper that returned (list name class) and a (mapcar ... that returned the-list-of-name-class-pairs. Then if any class objects were NIL, an error could be reported for those, probably conditional on an additional parameter to browse-class, and ask if to proceed. The ROOT-CLASSES would be constructed from the non NIL entries, i.e., (mapcan #'cdr the-list-of-name-class-pairs). The SETF of the browser TITLE should be the list of just the names of the successfully found classes. (If only one, then not as a list, to be consistent with the current behavior.) It also looks like a good idea to ensure that 'collect-familyandmake-nodesdetect (and ignore?) anyNILpassed in. (There are probably other places whereNIL` should be checked for.)

masinter commented 2 months ago

By the way, it might be a good idea to check if @Anzus has a revision of Medley CLOS... What I remember him saying was there were some problems in getting it to run, but it represented also bug fixes to the ... meata-object-protocol (?).