Skip to content

Conversation

@rmkaplan
Copy link
Contributor

More intuitive behavior when you call ED on something without the right package qualifier. E.g. if you (ED 'GET-INDENT) it now asks whether you want SEDIT::GET-INDENT instead of always bringing up the new-function-template menu.

Also, this removes the call to ERROR when it asks you whether you want to load a function to edit and you say No.

@pamoroso
Copy link
Contributor

On Linux Mint 22.1 Cinnamon this PR no longer generates the same error it does on master when saying no after ED asks whether to load a function:

7_ (ED 'CREATEW)
Editing FNS definition of CREATEW.
CREATEW not editable; do you want to LOADFROM PROP the file 
{DSK}<home>paolo>medley>medley>sources>WINDOW.;1 ? No
Could not find fns definition for CREATEW.
CREATEW

But (ED 'GET-INDENT) still brings up the new-function-template menu:

8_ (ED 'GET-INDENT)
GET-INDENT has no definitions.
Select a type of dummy definition to install.
NIL

@rmkaplan
Copy link
Contributor Author

ED does better when the OPTIONS argument includes the possible types. Try
(ED 'GET-INDENT 'FUNCTIONS)
or
(DF GET-INDENT)

This has been hanging around in my working directory since late last year. I thought I had also tried to set up a menu of types in addition to searching in a package-independent way. I'll look again.

@pamoroso
Copy link
Contributor

Including the type yields the expected behavior:

5_ (ED 'GET-INDENT 'FUNCTIONS)
SEDIT::GET-INDENT is contained on SEDIT-INDENT.
Shall I load this file PROP? N
SEDIT::GET-INDENT
6_ (DF GET-INDENT)
SEDIT::GET-INDENT is contained on SEDIT-INDENT.
Shall I load this file PROP? N
SEDIT::GET-INDENT

@rmkaplan
Copy link
Contributor Author

The symbol x type menu is what I was really aiming at last year. Matt reminded me that MKSTRING with flag T retained the package qualifer in the menu, but I never got back to it. If FIE exists in different packages with different types, the user can choose a symbol/type combination to edit.

@pamoroso
Copy link
Contributor

I updated to commit cea05f9 and (ED 'GET-INDENT) yields the error:

INTERLISP-ERROR
In ERROR:
DIVIDE-BY-ZERO
0
ed-error1
5_: BTV
   MESS1 "DIVIDE BY ZERO"
   MESS2 0
   NOBREAK NIL
ERROR
IQUOTIENT
GRIDYCOORD
   SI::*DUMMY-FOR-CATCH* T
   SI::*CATCH-RETURN-FROM* (& &)
MENU.HANDLERA0001A0003
   SI::*CLEANUP-FORMS* SI::RESETUNWIND
   MOUSEDOWN NIL
   MOVEDLEFT "NESTED"
   LASTBUTTONSTATE 0
   MGRIDSPEC (0 0 250 0)
   HOLDTIMER -1809309526
   HELDFN DEFAULTMENUHELDFN
   NROWS 0
   NCOLUMNS 1
   LOCALMENUHELDWAIT 1200
   ITEM NIL
   SUBITEMS NIL
   SUBMENURESULT NIL
   OLDBOXX NIL
   OLDBOXY NIL
   BOXX NIL
   BOXY NIL
   HELDSTATE NIL
   SUBMENUWINDOW NIL
   SUBMENU NIL
   SI::NLSETQ-VALUE NIL
   *PROCEED-CASES* (& &)
   SI::*NLSETQFLAG* NIL
SI::*UNWIND-PROTECT*
   MENU {MENU}#131,107130
   DSP #<Output Display Stream/167,122300>
   KEEPCONTROLIFOUTFLG T
   CHANGEOFFSETFLG T
   NESTEDFLG NIL
   LISPXHIST ((&) (3 "" . "_ ") "<not yet evaluated>" 
NIL)
   SI::*RESETFORMS* ((& NIL))
   RESETSTATE NIL
MENU.HANDLER
   SI::*CLEANUP-FORMS* SI::RESETUNWIND
SI::*UNWIND-PROTECT*
   MENU {MENU}#131,107130
   RELEASECONTROLFLG NIL
   NESTEDFLG NIL
   IMAGE {WINDOW}#156,165170
   DSP #<Output Display Stream/167,122300>
   LISPXHIST ((&) (3 "" . "_ ") "<not yet evaluated>" 
NIL)
   SI::*RESETFORMS* ((& {WINDOW}#156,165170))
   RESETSTATE NIL
MENU
ED
   *FORM* (ED (QUOTE GET-INDENT))
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* ED
\EVALFORM
FAULTEVAL
   *FORM* (UNDOABLY (ED &))
\EVALFORM
   \INTERNAL NIL
EVAL
EVAL-INPUT
   RETRYFLAG NIL
   HELPCLOCK 838
DO-EVENT
   SI::*DUMMY-FOR-CATCH* T
   SI::*CATCH-RETURN-FROM* (&)
   LISPXHIST ((&) (3 "" . "_ ") "<not yet evaluated>" 
NIL)
   HELPCLOCK 0
XCL::EXECA0001A0002
   *CURRENT-EVENT* ((&) (3 "" . "_ ") 
"<not yet evaluated>" NIL)
   SI::NLSETQ-VALUE NIL
   *PROCEED-CASES* (&)
   SI::*NLSETQFLAG* NIL
XCL::EXECA0001
\PROGV
   XCL::TOP-LEVEL-P T
   XCL::WINDOW {WINDOW}#156,165664
   XCL::TITLE-SUPPLIED NIL
   XCL::TITLE NIL
   *THIS-EXEC-COMMANDS* (#<Hash-Table @ 166,33666>)
   XCL::ENVIRONMENT NIL
   XCL::PROMPT NIL
   XCL::FN EVAL-INPUT
   XCL::PROFILE "XCL"
   *EXEC-ID* ""
   XCL::PROFILE-CACHE (XCL::*PROFILE-NAME* "XCL" 
XCL:*EVAL-FUNCTION* CL:EVAL *PACKAGE* #<Package XCL-USER> *READTABLE* #<ReadTable XCL/174,56634> XCL:*EXEC-PROMPT* "> " --)
EXEC
\PROC.REPEATEDLYEVALQT
   *FORM* (\PROC.REPEATEDLYEVALQT)
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* \PROC.REPEATEDLYEVALQT
\EVALFORM
   %#FORM# (\PROC.REPEATEDLYEVALQT)
   *CURRENT-PROCESS* #<Process EXEC/174,13204>
   HELPFLAG BREAK!
   \CURRENTDISPLAYLINE 0
   \#DISPLAYLINES 20
   \LINEBUF.OFD #<IO Linebuffer Stream/167,124600>
   *READTABLE* #<ReadTable INTERLISP/174,56714>
   \PRIMTERMTABLE {TERMTABLEP}#174,51740
   \PRIMTERMSA {CHARTABLE}#174,52000
   TtyDisplayStream #<Output Display Stream/146,32100>
   SI::*RESETFORMS* NIL
   \INTERRUPTABLE T
   \TTYWINDOW NIL
   READBUF NIL
   \TERM.OFD #<Output Display Stream/167,122500>
   *STANDARD-OUTPUT* #<Output Display Stream/167,122500>
   *STANDARD-INPUT* #<IO Linebuffer Stream/167,124600>
\MAKE.PROCESS0
T

Evaluating (DF GET-INDENT) yields the error:

INTERLISP-ERROR
unrecognized manager definition type
SEDIT::GET-INDENT
ed-error2
5_: BTV
   MESS1 "unrecognized manager definition type"
   MESS2 SEDIT::GET-INDENT
   NOBREAK NIL
ERROR
GETFILEPKGTYPE
GETFILEPKGTYPE
GETFILEPKGTYPE
   SOURCE NIL
EDITDEF
ED
   FN (GET-INDENT)
   *EDITMODE* DISPLAY
   ARGS (GET-INDENT)
DF
CL:FUNCALL
   *FORM* (CL:FUNCALL (FUNCTION DF) (QUOTE &))
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* CL:FUNCALL
\EVALFORM
FAULTEVAL
   *FORM* (UNDOABLY (DF GET-INDENT))
\EVALFORM
   \INTERNAL NIL
EVAL
EVAL-INPUT
   RETRYFLAG NIL
   HELPCLOCK 322
DO-EVENT
   SI::*DUMMY-FOR-CATCH* T
   SI::*CATCH-RETURN-FROM* (&)
   LISPXHIST ((&) (3 "" . "_ ") "<not yet evaluated>" 
NIL)
   HELPCLOCK 0
XCL::EXECA0001A0002
   *CURRENT-EVENT* ((&) (3 "" . "_ ") 
"<not yet evaluated>" NIL)
   SI::NLSETQ-VALUE NIL
   *PROCEED-CASES* (&)
   SI::*NLSETQFLAG* NIL
XCL::EXECA0001
\PROGV
   XCL::TOP-LEVEL-P T
   XCL::WINDOW {WINDOW}#156,165664
   XCL::TITLE-SUPPLIED NIL
   XCL::TITLE NIL
   *THIS-EXEC-COMMANDS* (#<Hash-Table @ 166,33666>)
   XCL::ENVIRONMENT NIL
   XCL::PROMPT NIL
   XCL::FN EVAL-INPUT
   XCL::PROFILE "XCL"
   *EXEC-ID* ""
   XCL::PROFILE-CACHE (XCL::*PROFILE-NAME* "XCL" 
XCL:*EVAL-FUNCTION* CL:EVAL *PACKAGE* #<Package XCL-USER> *READTABLE* #<ReadTable XCL/174,56634> XCL:*EXEC-PROMPT* "> " --)
EXEC
\PROC.REPEATEDLYEVALQT
   *FORM* (\PROC.REPEATEDLYEVALQT)
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* \PROC.REPEATEDLYEVALQT
\EVALFORM
   %#FORM# (\PROC.REPEATEDLYEVALQT)
   *CURRENT-PROCESS* #<Process EXEC/174,13204>
   HELPFLAG BREAK!
   \CURRENTDISPLAYLINE 0
   \#DISPLAYLINES 20
   \LINEBUF.OFD #<IO Linebuffer Stream/167,124600>
   *READTABLE* #<ReadTable INTERLISP/174,56714>
   \PRIMTERMTABLE {TERMTABLEP}#174,51740
   \PRIMTERMSA {CHARTABLE}#174,52000
   TtyDisplayStream #<Output Display Stream/146,32700>
   SI::*RESETFORMS* NIL
   \INTERRUPTABLE T
   \TTYWINDOW NIL
   READBUF NIL
   \TERM.OFD #<Output Display Stream/167,122500>
   *STANDARD-OUTPUT* #<Output Display Stream/167,122500>
   *STANDARD-INPUT* #<IO Linebuffer Stream/167,124600>
\MAKE.PROCESS0
T

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Sep 30, 2025 via email

@pamoroso
Copy link
Contributor

Commit a296c4e fixes the DF issue:

6_ (DF GET-INDENT)
SEDIT::GET-INDENT is contained on SEDIT-INDENT.
Shall I load this file PROP? Y

{DSK}<home>paolo>medley>medley>sources>SEDIT-INDENT.;1
File created 17-May-90 11:03:43
IL:SEDIT-INDENTCOMS

{DSK}<home>paolo>medley>medley>sources>SEDIT-DECLS.LCOM;1
compiled on  1-Dec-2021 23:12:16
File created  1-Dec-2021 20:02:36

EXPORTS.ALL must be loaded to compile SEdit

SEDIT-ACCESS must be REMADE NEW if you change a record
SEDIT::GET-INDENT

But I still get the same error with ED:

4_ (ED 'GET-INDENT)
In ERROR:
DIVIDE BY ZERO
0

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Sep 30, 2025 via email

@pamoroso
Copy link
Contributor

I confirm that, on this branch, passing the type as the second argument works as expected:

5_ (ED 'GET-INDENT 'FUNCTIONS)
SEDIT::GET-INDENT

In case it's useful I posted some notes on working with managed Common Lisp code on Medley.

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Sep 30, 2025 via email

@pamoroso
Copy link
Contributor

I updated to commit 46c3c08 and tested the APPEND symbol which has multiple types:

8_ (ED 'APPEND)
Edit which (PROPERTY-LIST OPTIMIZERS FNS) definition of APPEND ? FNS
APPEND

@rmkaplan
Copy link
Contributor Author

The APPEND case is the old behavior--the user has typed a symbol (with implicit package), and that symbol is known and has the various types of definitions.

The new behavior is for when the user doesn't know or bother with the package, or whether the symbol is internal or external. In that case it is supposed to offer a choice of all the symbols in any package (internal or external) that have definitions of any of the specified (if any) types. (If the user clicks away from the menu, it current brings up the old prototype menu--maybe it should do nothing in that case?)

If there is a single symbol with multiple types, it is supposed to bring up a menu with just the types of that symbol, with the title showing the package qualifier. There should be a difference between

(ED 'GET-INPUT NIL) (no types specified in the call, brings up a menu of SETFS and FUNCTIONS)
DF GET-INPUT (types specified as (FNS FUNCTIONS), goes immediately to the FUNCTIONS

@pamoroso
Copy link
Contributor

My head is:

paolo@lispmachine:~/medley/medley$ git log | head
commit 46c3c0898ff22ae5de8e7232ccc9bf1eb14c5e40
Author: rmkaplan <[email protected]>
Date:   Tue Sep 30 11:18:57 2025 -0700

    Fixed the menu case of one symbol with multiple types

commit a296c4ece87edba7e3eba6b9801c06a9d0b3adde
Author: rmkaplan <[email protected]>
Date:   Tue Sep 30 07:50:25 2025 -0700

But I still get the old behavior when no types are specified:

ed1 ed2

@rmkaplan
Copy link
Contributor Author

Try GET-INDENT instead of GET-INPUT ?

@pamoroso
Copy link
Contributor

pamoroso commented Oct 1, 2025

GET-INDENT yields the new behavior:

ed3

@pamoroso
Copy link
Contributor

pamoroso commented Oct 2, 2025

I updated to commit f91e993 and when I call (ED 'MYFUN) (or (ED 'MYFUN '(FNS :DONTWAIT))) and click on FNS > LAMBDA in the type menu I get this error:

ATTEMPT-TO -RPLAC-NIL
In /PUTPROP:
Attempt to rplac NIL with (NIL EXPR)
ed-error
5_: BTV
   ATM NIL
   PROP EXPR
   VAL (LAMBDA ("Arg List") "Body")
/PUTPROP
FNS.PUTDEF
DEFINE
DEFINEQ
   *FORM* (DEFINEQ (NIL &))
\EVALFORM
   \INTERNAL NIL
EVAL
   DFNFLG PROP
"MAKE-AND-INSTALL"
INSTALL-PROTOTYPE-DEFN
ED
   *FORM* (ED (QUOTE MYFUN))
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* ED
\EVALFORM
FAULTEVAL
   *FORM* (UNDOABLY (ED &))
\EVALFORM
   \INTERNAL NIL
EVAL
EVAL-INPUT
   RETRYFLAG NIL
   HELPCLOCK 1402
DO-EVENT
   SI::*DUMMY-FOR-CATCH* T
   SI::*CATCH-RETURN-FROM* (&)
   LISPXHIST ((&) (3 "" . "_ ") "<not yet evaluated>" 
NIL SIDE (1 &))
   HELPCLOCK 0
XCL::EXECA0001A0002
   *CURRENT-EVENT* ((&) (3 "" . "_ ") 
"<not yet evaluated>" NIL SIDE (1 &))
   SI::NLSETQ-VALUE NIL
   *PROCEED-CASES* (&)
   SI::*NLSETQFLAG* NIL
XCL::EXECA0001
\PROGV
   XCL::TOP-LEVEL-P T
   XCL::WINDOW {WINDOW}#156,166664
   XCL::TITLE-SUPPLIED NIL
   XCL::TITLE NIL
   *THIS-EXEC-COMMANDS* (#<Hash-Table @ 166,32666>)
   XCL::ENVIRONMENT NIL
   XCL::PROMPT NIL
   XCL::FN EVAL-INPUT
   XCL::PROFILE "XCL"
   *EXEC-ID* ""
   XCL::PROFILE-CACHE (XCL::*PROFILE-NAME* "XCL" 
XCL:*EVAL-FUNCTION* CL:EVAL *PACKAGE* #<Package XCL-USER> *READTABLE* #<ReadTable XCL/174,56634> XCL:*EXEC-PROMPT* "> " --)
EXEC
\PROC.REPEATEDLYEVALQT
   *FORM* (\PROC.REPEATEDLYEVALQT)
   *ARGVAL* NIL
   *TAIL* NIL
   *FN* \PROC.REPEATEDLYEVALQT
\EVALFORM
   %#FORM# (\PROC.REPEATEDLYEVALQT)
   *CURRENT-PROCESS* #<Process EXEC/174,13204>
   HELPFLAG BREAK!
   \CURRENTDISPLAYLINE 3
   \#DISPLAYLINES 20
   \LINEBUF.OFD #<IO Linebuffer Stream/167,124400>
   *READTABLE* #<ReadTable INTERLISP/174,56714>
   \PRIMTERMTABLE {TERMTABLEP}#174,51740
   \PRIMTERMSA {CHARTABLE}#174,52000
   TtyDisplayStream #<Output Display Stream/146,41000>
   SI::*RESETFORMS* NIL
   \INTERRUPTABLE T
   \TTYWINDOW NIL
   READBUF NIL
   \TERM.OFD #<Output Display Stream/167,124200>
   *STANDARD-OUTPUT* #<Output Display Stream/167,124200>
   *STANDARD-INPUT* #<IO Linebuffer Stream/167,124400>
\MAKE.PROCESS0
T

@pamoroso
Copy link
Contributor

pamoroso commented Oct 2, 2025

Commit 60a6f0a fixes the issue.

@masinter
Copy link
Member

masinter commented Oct 3, 2025

My problem isn't so much the interaction with the file manager. Apart from the UNSAVEDEF, what makes it hard to debug is that I can't set up a call to HELP, get a break window, and then copy expressions from the code into the break to do step-wise, form-at-a-time debugging. None of the local variables are bound, even when running the code symbolically. Isn't that supposed to work?

@rmkaplan this works in simple cases:

(defun test (x y z) (/ y z))
(test 'x-value 333 0)
retry
in the break:

x. => X-VALUE
Y => 333
Z. => 0

it picks up the lexical context from the CL:EVAL call.
I'm sure there are cases where things don't work that way
because the lexically scoped variables are really not visible
at the break.

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Oct 3, 2025

I appears that this only works if the break happens under an XCL exec, not an Interlisp exec. If a defun is called somewhere under an Interlisp exec and it gets an error break or it calls HELP, the variables appear not to be bound.

@MattHeffron MattHeffron requested a review from hjellinek October 6, 2025 17:37
Copy link
Contributor

@hjellinek hjellinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before I officially "review" this per GitHub, I need to know what this is promising.
I tried my usual define/edit flow, in which I defined a function by
ED(MY-NEW-FUNCTION)
then selecting FUNCTIONS from the pop-up and defining the function in SEdit. That worked fine - no change from the behavior in master.
Editing the function, now that it exists, also worked as it did before:
ED(MY-NEW-FUNCTION

HTMLSTREAM declares U-PNG as a dependency using FILES.
I loaded HTMLSTREAM.DFasl, after which U-PNGCOMS has the expected value.

The file U-PNG defines a package, also named U-PNG, with a mix of public (e.g., WRITE-BITMAP) and internal (e.g., TEST-IT) symbols. I was hoping that ED would now be able to find those without further work.

In an Interlisp exec I typed
ED(WRITE-BITMAP)
and got the "dummy def'n" popup. I received the same result for
ED(TEST-IT)

GETD returns Compiled Function objects for both.

I went into the FileBrowser and did an explicit LOAD PROP of U-PNG. Now things worked as I had hoped: both ED(WRITE-BITMAP) and ED(TEST-IT) found the symbols in the U-PNG package and edited the expected definittions.

So ED did not intuit the need to ask me whether to LOAD U-PNG PROP. If this PR was supposed to enable that, it didn't succeed. If not, then I'd say it works fine (but I wish it had the extra logic I envisioned), and I will approve it.

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Oct 6, 2025

@hjellinek , all (or many) bets are off with DFASL files. As noted in #2310 , they are implicitly loaded SYSLOAD, so the coms don't exist after the load. If your functions are not in WHEREIS, then it is not known where their editable definitions are until you load the symbolic file.

It may be that the DFASL loader can be fixed up so that it interacts better with the FILEPKG and managed files. But it seems that this is another example of only partial integration of common Lisp with the Medley residential programming style. What would happen if we used FAKE-COMPILE-FILE for all managed files and reserved CL:COMPILE-FILE for unmanaged pure common Lisp text files?

@hjellinek
Copy link
Contributor

@hjellinek , all (or many) bets are off with DFASL files. As noted in #2310 , they are implicitly loaded SYSLOAD, so the coms don't exist after the load. If your functions are not in WHEREIS, then it is not known where their editable definitions are until you load the symbolic file.

It may be that the DFASL loader can be fixed up so that it interacts better with the FILEPKG and managed files. But it seems that this is another example of only partial integration of common Lisp with the Medley residential programming style. What would happen if we used FAKE-COMPILE-FILE for all managed files and reserved CL:COMPILE-FILE for unmanaged pure common Lisp text files?

Relegating CL:COMPILE-FILE to only handling unmanaged pure CL source code would mean losing functionality. Loading DFASL files directly works as you'd expect w/r/to the file manager. As I mentioned in my comments, the COMS did exist after the FILES load; they simply weren't consulted. When I load a DFASL directly, all the file manager stuff works fine.

I'm also not sure how common it is to try to compile pure CL source code.

But it seems that loading a DFASL via FILES wasn't expected to work as I'd hoped, so my testing verifies that this PR works.

Copy link
Contributor

@hjellinek hjellinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rmkaplan explained the goal of this PR, and it does seem to work with my workflow. It improves the user experience, so I will approve it.

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Oct 6, 2025

It appears that LOAD(HTMLSTREAM.DFASL) does make the FUNCTIONS on that file known to the file package and therefore to ED.

But HTMLSTREAM loads U-PNG and BASE64 SYSLOAD, so the items on those files are not known.

@masinter
Copy link
Member

masinter commented Oct 6, 2025 via email

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Oct 6, 2025 via email

@masinter
Copy link
Member

masinter commented Oct 6, 2025

What seems to be happening is that BASE64.DFASL is loaded, its coms are there, but it is not on FILELST

How does that happen?

@rmkaplan rmkaplan merged commit f6106d7 into master Oct 7, 2025
@hjellinek
Copy link
Contributor

What seems to be happening is that BASE64.DFASL is loaded, its coms are there, but it is not on FILELST

How does that happen?

I do not know, but it would be great if it worked as I had expected it to.

@pamoroso
Copy link
Contributor

pamoroso commented Oct 8, 2025

Is the issue with BASE64.DFASL and FILELST related to discussion #2308? WEB-EDITOR is a managed file.

@rmkaplan
Copy link
Contributor Author

rmkaplan commented Oct 8, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants