spy16 / sabre

Sabre is highly customisable, embeddable LISP engine for Go. :computer:
GNU General Public License v3.0
28 stars 5 forks source link

export more internal functions from specials.go #23

Open leifj opened 4 years ago

leifj commented 4 years ago

It is currently impractical to build specials outside sabre (eg my case implementation in #22) because functions like 'analyze' are not exported.

lthibault commented 4 years ago

Hi @leifj,

100% agree with this. It was brought up in a previous issue (which I can't seem to find for some reason), and I think @spy16 was also onboard.

My off-the-cuff suggestion is to approach this from a "toolchain" perspective, and create a subpackage of common functionality on top of which Sabre is implemented. Perhaps we could call it something like core. Any input or ideas would be most valued and appreciated, as would pull-requests.

@spy16 This ok with you?

leifj commented 4 years ago

Yeah that would work I guess

spy16 commented 4 years ago

Specials handling right now is far from ideal. I do have some ideas to improve this. But haven't been able to get the time to do it.

I simply decided to leave the analyse private since doing it in an extendible way with support for custom data types would add some complexity. And also assumed there's just enough to build everything else using either macros or functions.

For example implementing case construct would be possible through macros as well and i would like to understand why you chose/preferred to do it as a special form.

leifj commented 4 years ago

I guess I never considered a macro - could you demonstrate?

leifj commented 4 years ago

Incidentally I am in the process to switch over to using slang as a dependency for github.com/SUNET/tq instead of vanilla sabre to get access to more of the lisp primitives. I'm perfectly happy if the answer is "implement in slang".

spy16 commented 4 years ago

@leifj Actually, i implemented using Go itself but not special-form or macro. Take a look: Case function is defined here: https://github.com/spy16/slang/blob/master/core.go#L13

This is registered into the scope after wrapping inside an Fn:

&sabre.Fn{
    Args:     []string{"exprs", "clauses"},
    Func:     Case,
    Variadic: true,
}

This is in a way special since Case function gets un-evaluated list of args and the current scope. But it's not a special form because it doesn't get analyzed and verified when nested within other lists.

spy16 commented 4 years ago

@lthibault While the above approach might work for @leifj , i agree that it is probably best to re-organise the codebase. Any particular structure in mind?

I used a single package initially since i couldn't figure out a way to split things into packages while making sure the usages of sabre remains sabre.X in most cases which is preferable i think because in end-user's codebases it might be confusing to have core.Eval etc. since core is common name and also doesn't communicate it's coming from sabre. One other approach i had in mind was to move all the Value types into a value or core package. sabre can then expose useful functions like Eval, Analyse etc. (This way, at least the simpler use cases remain sabre.Eval, sabre.ReadEval etc.)

lthibault commented 4 years ago

@spy16 Agreed on all counts. +1 for subpackage value.

leifj commented 4 years ago

@spy16 this approach definitely works for me and given that I am now depending on slang I am going to close my PR

spy16 commented 4 years ago

@lthibault I have started working on this. I am also taking into consideration some other shortcomings i have been thinking about (For example no control over evaluation rules because many container types invoke sabre.Eval function directly) and making certain design changes. ..

Some highlights of the changes i am making:

  1. core package contains the core interface definitions and core data structures (like List, Symbol, Keyword, String etc. but probably not Fn etc.)..

  2. core also contains interface Env (replaces Scope) and moves the sabre.Eval into Env. (This allows custom environments where even evaluation rules can be overridden)

    type Env interface {
    Eval(form Value) (Value, error)
    Bind(symbol string, v Value) error
    }
  3. This refactor might also address #16 and #17 .

In summary, this is inline with your suggestion of having a core package with core functionality and the sabre package building on top of it i think.

(Did not go with the value package idea exactly as discussed above because of cyclic dependencies.)

lthibault commented 4 years ago

Cool! That all sounds good.

I should be available to do code review in the coming days, if it's needed.