atlas-engineer / nfiles

User configuration and data file management
BSD 3-Clause "New" or "Revised" License
17 stars 5 forks source link

Use keywords instead of unbound symbols for slot options #14

Closed Ambrevar closed 1 year ago

Ambrevar commented 1 year ago

For instance in

(on-external-modification
    'ask
    :type (member ask reload overwrite)
    :documentation "Whether to reload or overwrite the file if it was modified
since it was last loaded.")

...

(export-always '(ask reload overwrite))

We export unbound symbols ask, reload and overwrite. This is not very good practice. Switching to keywords would be cleaner.

The only benefit of using symbols is that they allow for completion, but since they are not in a separate package anyways, it's unlikely that completion is ever going to be used.

Unfortunately this breaks backward compatibility. I suggest to release 2.0 then, since a new version is long due.

@aadcg @aartaka ?

aartaka commented 1 year ago

Yes, I'm all for keywords :D

New version is okay too!

aadcg commented 1 year ago

Good idea.

Ambrevar commented 1 year ago

So I suggest we first release 1.1.0 as per #13 (@aadcg will do it I think), then we can release 2.0.0.

Ambrevar commented 1 year ago

Actually I remember why I went with symbols: it's because I initially thoughts that restarts had to be non-keyword symbols. But that's not true, since this works:

(restart-case
     (let ((r (find-restart :my-restart)))
       (format t "~S is named ~S" r (restart-name r)))
   (:my-restart () nil))

So really no reason to keep these non-keyword symbols.

aartaka commented 1 year ago

As an alternative perspective, maybe we can add functions that mirror/invoke the given restart? The standard even has some functions like that (continue, abort). So we can retain those symbols and bind those to functions like that:

(defun ask (&optional condition)
  (invoke-restart (find-restart 'ask condition)))
Ambrevar commented 1 year ago

I like it!