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

Silly-j parser #12

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Silly-j parser #12

wants to merge 6 commits into from

Conversation

hysenbug
Copy link
Collaborator

This is a basic implementation about silli-j.

SPC = #'\\ '+
APPLY = #'^[^@\\ ][^}\\ ]+'
"
))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Odie I think that you might want to give instaparse a try.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I think mustache is sufficiently simple to deal with by hand. (Famous last words?)

Instaparse is really fantastic, but my last impression of it was “really magical, but maybe somewhat slow”. It might have been due to an ambiguous grammar that caused it to try going down two separate paths of the grammar tree though.

Anyway, since there are no complicated grammatical structures, and mustache is already fully spec’ed out, I’ll try seeing it through the rest of the way.

(:require [hopen.renderer.xf :as rxf]
[instaparse.core :as insta]))

(declare parse)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be avoided if the render function can be defined after the parser.

Usually, the declare is only used when there is a cross recursion between 2 functions defined at the root level.

(declare parse)

;; TODO test case
(defn render
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to let the user do his own composition between the renderers and the parsers.

If this function is here just for testing, it makes sense to move it to the test namespace.

STR = #'[^{}]+'
SILLYSTR = OBK SILLY CBK
OBK = '{'
CBK = '}'
Copy link
Member

@green-coder green-coder Apr 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest using non-abbreviated names or less abbreviated names, to ensure that anybody can easily read the source code.

OPEN_BLOCK, CLOSE_BLOCK for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note: the casing (upper-case / lower-case) does not matter for instaparse.

FN = FNNAME APPLIES*
APPLIES = SPC | FCTX | APPLY
FNNAME = #'[a-zA-Z0-9\\-]+'
SPC = #'\\ '+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SPACE

@green-coder
Copy link
Member

@hylisd So far, so good. I am impressed by the small size of the code.

More tests would be needed, to parse different kind of forms in the templates.

# Conflicts:
#	test/hopen/runner.cljs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants