atlas-engineer / nfiles

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

Switch slot options from symbols to keywords. #19

Closed Ambrevar closed 1 year ago

Ambrevar commented 1 year ago

This saves us from exporting unbound symbols, which is poor and confusing practice.

Fixes #14.

Ambrevar commented 1 year ago

@aartaka @aadcg OK to merge? Then I'll release 2.0.0.

aadcg commented 1 year ago

All good from my side!

aartaka commented 1 year ago

Now that I look at it in the light of https://github.com/atlas-engineer/nfiles/issues/14#issuecomment-1431310848, I'm in doubt of whether we should limit ourselves with keywords. Here's what we can do instead, preserving backwards-compatibility and allowing for more extensibility:

So the use of the library could then be:

I mean, no one forbids doing this with keywords too, but that looks quite weird to name restarts with keywords...

Ambrevar commented 1 year ago

Yes, this is better, + no need for a 2.0.0.

  • Change the type of those restart slots to symbol.

No need I believe. If the user extends the class and the slot, then they can redefine the type to (member NEW-RESTART OLD-RESTART).

Ambrevar commented 1 year ago

See #20.

aartaka commented 1 year ago
  • Change the type of those restart slots to symbol.

No need I believe. If the user extends the class and the slot, then they can redefine the type to (member NEW-RESTART OLD-RESTART).

This should work per standard, yes. But then, SBCL is extremely cautious about slot types at (safety 3), so it won't allow to redefine the type to be looser than the previous one. So we'd better account for these cases in advance and use symbol by default. Yes, it's quite vague, but at least it's vague enough to not create type mismatches in the compiler.

Ambrevar commented 1 year ago

Good point!