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.

Sign up for GitHub

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

Jump to bottom

Livescript implementation #262

Merged
merged 19 commits into from
May 24, 2017
Merged

Livescript implementation #262

merged 19 commits into from
May 24, 2017

Conversation

c0deaddict
Copy link

I've recently followed the mal guide to build an implementation in Livescript. It was really much fun! Thanks for the great guide!

I noticed there is an open pull request for another Livescript implementation, but the last comments on that thread are from 2015. So I thought I'll give it a try.

What do you think about my Livescript implementation?

@kanaka
Copy link
Owner

kanaka commented May 24, 2017 •
edited
Loading

Implementation looks good! Glad you enjoyed it. I've verified that both regular and self-hosted tests pass. However, there are a few minor things before I merge:

  • Running the following from the top-level should work: make -C livescript clean && make test^livescript. In order to make that work there are some missing dependencies in the Makefile:
diff --git a/livescript/Makefile b/livescript/Makefile
index 080d572..07c91d2 100644
--- a/livescript/Makefile
+++ b/livescript/Makefile
@@ -13,8 +13,19 @@ all: node_modules $(BINS)
 node_modules:
        npm install
 
-%.js: %.ls
+%.js: %.ls node_modules
        $(LSC) -d -c $(@:%.js=%.ls)
 
+step1_read_print.js: utils.js reader.js printer.js
+step2_eval.js: utils.js reader.js printer.js
+step3_env.js: utils.js reader.js printer.js env.js
+step4_if_fn_do.js: utils.js reader.js printer.js env.js core.js
+step5_tco.js: utils.js reader.js printer.js env.js core.js
+step6_file.js: utils.js reader.js printer.js env.js core.js
+step7_quote.js: utils.js reader.js printer.js env.js core.js
+step8_macros.js: utils.js reader.js printer.js env.js core.js
+step9_try.js: utils.js reader.js printer.js env.js core.js
+stepA_mal.js: utils.js reader.js printer.js env.js core.js
+
 clean:
        rm -f $(BINS)
  • Please add the following Dockerfile to your directory:
FROM ubuntu:xenial
MAINTAINER Joel Martin <[email protected]>

##########################################################
# General requirements for testing or common across many
# implementations
##########################################################

RUN apt-get -y update

# Required for running tests
RUN apt-get -y install make python

# Some typical implementation and test requirements
RUN apt-get -y install curl libreadline-dev libedit-dev

RUN mkdir -p /mal
WORKDIR /mal

##########################################################
# Specific implementation requirements
##########################################################

# For building node modules
RUN apt-get -y install g++

# Add nodesource apt repo config for 7.X
RUN curl -sL https://deb.nodesource.com/setup_7.x | bash -

# Install nodejs
RUN apt-get -y install nodejs

  • Activate your build for travis testing by adding it to .travis.yml. Once you do that, the next time you push to github, travis should test your implementation. I've already built and pushed a docker image built using the above Dockerfile to the docker hub so it should be able to be tested as soon as it's enabled.

  • Add your implementation to the README citing yourself as the creator (see typescript as an example). Also, bump the implementation count.

  • [Optional] Add stats and stats-lisp rules to your Makefile. coffee/Makefile probably has a pretty close rules for counting comment/blank lines. I can do this afterwards if you don't want to.

  • Push your changes. Once travis confirms everything passes, then I'll merge.

  • [Optional] Write a blog post about your experience and interesting things you learned. This is totally optional of course, but if you were thinking about doing this anyways, don't wait too long or you'll start to forget things you learned that might be worth sharing.

  • Send a tweet about your implementation (with a link to blog post if you decide to do that). Link to your tweet here and I'll retweet it.

  • [Optional] Join #mal on freenode. It's pretty quiet but there is occasional interesting discussion of mal and Lisp related topics.

@dubek
Copy link
Collaborator

dubek commented May 24, 2017 via email

@c0deaddict
Copy link
Author

Thanks for your feedback @kanaka. I've implemented the changes. I'm not quite sure what the regex does in stats and stats-lisp, but I copied it from the coffeescript implementation and it appears to work.

I'll send a tweet later tonight, and i'll think about making a blog post. Thanks again for this wonderful project. I'm thinking about doing another implementation in another language :-)

@kanaka kanaka merged commit 04260e2 into kanaka:master May 24, 2017
@kanaka
Copy link
Owner

kanaka commented May 24, 2017

I stopped Travis for the other implementations (so we don't have to wait a couple hours). I had to make a minor Dockerfile update, but now things pass: https://travis-ci.org/kanaka/mal/jobs/235731614

@c0deaddict
Copy link
Author

https://twitter.com/c0deaddict/status/867659519521943553

@kanaka
Copy link
Owner

kanaka commented May 25, 2017

@c0deaddict I note that you have some Elm background. Surprisingly, Elm is currently missing as a mal implementation. Interested in tackling that next? You'll might have to deal with some hassle around command line execution. E.g. https://github.com/jfairbank/run-elm

@c0deaddict
Copy link
Author

That would be interesting, i'm on it!

@kanaka kanaka mentioned this pull request Sep 13, 2017
Closed
micfan pushed a commit to micfan/make-a-lisp that referenced this pull request Dec 2, 2018
@kanaka
Merge pull request kanaka#262 from c0deaddict/master
9cb962e 
Livescript implementation
luelista pushed a commit to luelista/mal that referenced this pull request Mar 10, 2024
@kanaka
Merge pull request kanaka#262 from c0deaddict/master
ebe3ff5 
Livescript implementation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@c0deaddict @kanaka @dubek