Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

expose readtables to users #687

Open
disnet opened this issue Apr 17, 2017 · 8 comments
Open

expose readtables to users #687

disnet opened this issue Apr 17, 2017 · 8 comments

Comments

@disnet
Copy link
Member

disnet commented Apr 17, 2017

Readtables are used internally but currently there is no way for users to extend them.

I'd like to use the directive lang <path> to do the exposing but beyond that I haven't thought too much about the API yet.

@gabejohnson besides figuring out the user surface API is there anything you know we need to handle first?

@gabejohnson
Copy link
Member

We should get the reader at least not hanging on 262 tests (there's a list of them that appear to get stuck).

I also don't like the current API. CharStream seems unnecessary unless it's just there to hold state (which I don't like). I have some ideas that I've been meaning to write up in the readtables repo. I'll take a look at it this week and ping you for feedback.

@gabejohnson
Copy link
Member

Also we should figure out what #519 is going to look like.

@gabejohnson
Copy link
Member

#707 and #712 have been just the most recent of many request for custom punctuators. I'm rethinking my stance on exposing the API. While I don't want to upset users by changing the readtable API in the near future, I think exposing it using the current implementation could be very fruitful in exposing pain points.

@disnet what do you think?

@disnet
Copy link
Member Author

disnet commented May 31, 2017

@gabejohnson I think that's totally reasonable. We'd probably be fooling ourselves if we thought we could get away without some iteration on the API anyway. We should be sure an note in the documentation the API is experimental and will probably change.

@samuelgoto
Copy link

It seems like this was introduced and then has gone away. Just checking: is it deliberate that this documentation for the readtable is outdated? if so, is there a replacement?

@disnet
Copy link
Member Author

disnet commented Jul 31, 2017

@samuelgoto correct. Readtables were exposed in version 0.7 and then went away in the 1.0 rewrite. @gabejohnson has been working on their replacement. Like this issue mentions, Sweet is currently using @gabejohnson's readtables internally and we need to figure out the right way of exposing them to users.

@samuelgoto
Copy link

samuelgoto commented Jul 31, 2017 via email

@disnet
Copy link
Member Author

disnet commented Aug 1, 2017

In the interim, is there a quick hack that you'd recommend to access them
(forking and digging in is an option, but sounds undesirable/brittle)?

Yeah, the reader is being run here for each module. The readtable is mutable and modified with setCurrentReadtable (e.g. here) so right before read is called you can dispatch to however you want to hack in your custom readtable.

On a side note, what is the best communication channel for me to ask some
quick questions about feasibility of new syntax (github issues? IRC?
hangouts? mailing list?) and whether sweet.js is the right tool for it?

Github issues work best for me. The gitter room is active occasionally but I'm pretty infrequent there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants