spy16 / parens

Parens is a highly flexible and embeddable LISP toolkit. :computer:
33 stars 3 forks source link

Fix multiple papercuts #9

Closed lthibault closed 4 years ago

lthibault commented 4 years ago

Hi @spy16,

I've been developing a small language using my own fork of Parens. In doing so, I've made the following changes, which I think should be merged upstream.

If any of these changes seem absurd, please let me know.

Changes

1. Add ReaderError.Unwrap method

ReaderError contains a Cause field. Since Go 1.13, the errors package contains errors.Unwrap, which can retrieve the cause of any error matching the following interface:

type unwrapper interface {
    Unwrap() error
}

This feature is also supported by github.com/pkg/errors, which in turn is officially recommended by the Go team (Exhibit A, Exhibit B).

2. Wrap strconv.ParseFloat and strconv.ParseInt errors with extra debug info

Parens currently fails to parse e.g. integers above 9223372036854775807 due to overflow. Currently, this returns an extremely unhelpful illegal number format error. This has been augmented by the github.com/pkg/errors package mentioned above.

case decimalPoint:
    v, err := strconv.ParseFloat(numStr, 64)
    if err != nil {
        // return nil, fmt.Errorf("illegal number format: '%s'", numStr)
        return nil, errors.Wrap(err, "illegal number format")
    }
    return Float64(v), nil
default:
    v, err := strconv.ParseInt(numStr, 0, 64)
    if err != nil {
        // return nil, fmt.Errorf("illegal number format '%s'", numStr)
        return nil, errors.Wrapf(err, "illegal number format %s", numStr)
    }

    return Int64(v), nil

This provides a major advantage when debugging for two reasons:

  1. The default error message is more informative
  2. We can print a detailed stack trace via fmt.Printf("%+v", errors.Unwrap(err)).

3. Make numbers, strings, keyword and chars evaluate to themselves

Currently, typing [ "foo" "bar" ] at the REPL will print [foo bar]. The output should instead be identical to the input. Since you seemed to be emulating Clojure in https://github.com/spy16/parens/issues/4, used https://clojure.org/reference/evaluation as a reference for correcting this.

It states:

Strings, numbers, characters, true, false, nil and keywords evaluate to themselves.

Note that it may be worth implementing a Bool type, but I have not done so.

4. Remove the leading : from the data representation of a keyword

The leading : in a keyword should not be part of the data, consistent with how characters and quoted forms are handled.

I made the appropriate modifications, ensuring that keywords are still properly printed and parsed.

spy16 commented 4 years ago

Merged. Thanks for your contributions.. (I had plans to remove these issues and few others while building Sabre and make that the main project and archive this. But for now i will leave this as is)