[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
one last (?) package question
I would like to point out a few things which I think are not the usual
way to do things, and suggest how I would do them.
Date: Tue, 5 SEP 89 19:35:47 PDT
From: KARNICKY@CRVAX.SRI.COM (Karnicky)
Change#0: You didn't say whether you did this or not, so I am assuming
you didn't. The file you sent should be the "system source file", as
selected by the function SCT:SET-SYSTEM-SOURCE-FILE. That is, there
should be a file named SYS:SITE;ZELDA.SYSTEM which contains the
following forms:
;;; -*- Mode: LISP; Base: 10; Package: USER; Syntax: Common-lisp -*-
(unless (fs:get-logical-pathname-host "ZELDA" t)
(fs:make-logical-pathname-host "ZELDA"))
(sct:set-system-source-file "ZELDA" "ZELDA:ZELDA;ZELDA-SYSTEM")
========================================================================
[I strongly recommend the use of logical pathnames in all system
definitions. The "UNLESS" above is my own personal cliche to prevent
the function MAKE-LOGICAL-PATHNAME-HOST from complaining if you happen
to have more than one system which uses the same logical pathname host,
and is not required -- If Symbolics would remove the bug which causes
M-L-P-H from complaining, I would not complain...]
The following forms (as modified by my later changes) should be in the
file ZELDA:ZELDA;ZELDA-SYSTEM.LISP:
;;; -*- Mode: LISP; Base: 10; Syntax: Common-lisp -*-
(defpackage zelda
(:prefix-name "ZELDA")
(:use symbolics-common-lisp)
(:export u d tt db l s h sd m st restart w e eplan editdb findme))
Change#1: I would say (:export "U" "D" "TT" ...) instead of using
symbols here, as barmar suggested. I also don't think it's very wise to
export such short, unmnemonic names from packages in general.
(defvar zelda::*task-directory nil "directory for jfk task plist files")
(setq zelda::*task-directory "vax:[jfk.zelda]")
Change#2: The forms above belong in one of the files of the system,
rather than in this file, usually.
;;; defines the TASKS file system for the LISPM
(defun mkzelda ()
;; No interesting value is returned
Change#3: This is really, really wrong. You shouldn't embed a DEFSYSTEM
inside a function. There isn't any good reason for doing this,
especially since you are only going to call that function once, here in
the file. Just put the DEFSYSTEM at top level.
(defsystem zelda
(:short-name "ZELDA"
:pretty-name "Task Management System"
:default-pathname "vax:[jfk.zelda]"
:journal-directory "vax:[jfk.zelda]"
Change#4: Use logical pathnames. This will save you a great deal of
time, effort and depression later, especially if you ever have to move
off a particular file host or go to bring up your software at another
site.
:patchable nil)
(:module init ("zelinit" "vax:[ai.vis]vrcinit"))
(:module newlispfns ("vax:[ai.vis]newlispfns" "vax:[ai.vis]store"
"vax:[ai.vis]util" "vax:[ai.vis]screenops")
(:in-order-to :compile (:load init)))
Change#5: you should probably make these files in [AI.VIS] into a
separate system. They can be a subsystem defined in this file if they
don't get used in another system as well, but the mere fact that they
are in a different directory not under [JFK] makes me think that the
proper modularization requires another system. As a bonus for putting
them into their own system, you get to supply a new default pathname,
which means you only need to mention (the logical pathname with which
you would replace) VAX:[AI.VIS] once.
Change#6: I suspect "ZELINIT" is not required in order to compile the
files in the module NEWLISPFNS; you should not make it part of the
compilation constraints if that suspicion is true.
(:module taskfns ("taskfns"
"maketfns"
"recurring"
"tasklispmfns"
"zeldevfns"
"zelsearch" "zeltime"
"zelutil" "zelhelp")
(:in-order-to :compile (:load init)
(:load newlispfns))); some macros here
(:module graphics ("zelgraph"
)
(:in-order-to :compile (:load init)
(:load newlispfns)))
) ;defsystem
(compile-system 'zelda :update-directory nil :increment-version nil
:query nil)
Change#7: This does not belong in the system declaration file. You
should just change things around as I mentioned above in Change#0, and
then use the "Compile System" CP command. If you never want to update
the version number or component directory, you should be using
non-patchable or non-journalized systems. I frequently do this for
systems which are still in early stages of development. Put
:MAINTAIN-JOURNALS NIL into the DEFSYSTEM form.
(format t "~%~%The TASKS system is now defined"))
;;;top level calls to make the system
(mkzelda)
[You can take this out now...]
(use-package 'zelda 'user)
Change#8: I think it's wrong to make the USER package use ZELDA. If
there are symbols you wish to appear in the USER package, you should
GLOBALIZE them there, or import them by hand. I suspect that rather
than wanting the symbols to appear in the USER package, you want them to
appear in whatever package you are typing. Say:
(mapc #'(lambda (x) (globalize (string x) 'SCL))
'(u d tt db l s h sd m st restart w e eplan editdb findme))
instead.
(in-package 'zelda)
Change#9: the above is not really what you want, especially since I am
about to tell you how to get rid of the following two forms.
(set-up-windows) ;defined in taskfns.lsp
(startt) ;read in tasks from file
Change#10: The above look like they are things you want to do
immediately after loading the Zelda system. They should be
:AFTER-PATCHES-INITIALIZATIONS, probably. This is a little complicated,
but here is a "formula" for it:
In the DEFSYSTEM file, say:
(defpackage ZELDA
...
(:export ... *ZELDA-INITIALIZATIONS*)
...)
(defsystem ZELDA
(...
:after-patches-initializations zelda:*zelda-initializations*
...)
...)
[Note: *ZELDA-INITIALIZATIONS* does not absolutely have to be exported
from the ZELDA package; I only do it because I prefer not to see "::"
anywhere in my code, even in the DEFSYSTEM form.]
Then, in TASKFNS.LISP, say
(add-initialization "Initialize windows" '(set-up-windows)
() *zelda-initializations*)
and wherever STARTT gets defined, add
(add-initialization "Read in tasks from file" '(startt)
() *zelda-initializations*)
:AFTER-PATCHES-INITIALIZATIONS get run even if your system isn't
patchable, I think (but try it; the old name for that option was just
:INITIALIZATIONS, which still works, which makes me think that
:AFTER-PATCHES-INITIALIZATIONS probably works).
(in-package 'user)
Change#11: This form will be unnecessary after the preceding three forms
are taken out.