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

automatically puts list around functions in threading macros if absent #132

Merged
merged 2 commits into from Jan 3, 2019

Conversation

@noisesmith
Copy link
Contributor

commented Jan 1, 2019

This replicates the familiar behavior of the threading macros with Clojure.

@technomancy

This comment has been minimized.

Copy link
Collaborator

commented Jan 2, 2019

I guess the only reason this wasn't included in the first pass was that while it's easy to add it in, it's impossible to remove later on if we decide it's not a good idea. Because of this, we should think a little more about whether it's the right thing for Fennel and not just do it because it can be done easily.

I don't feel strongly about it either way, but to me the advantage of the current way is that the code looks closer to its actual meaning. To people who haven't already used Clojure, it's easier to see the correspondence between the input and the resulting macroexpansion when the parens are there. The downside is ... slightly more typing, I guess.

In Clojure, you commonly see forms like (-> m :key :word :inside) because keywords are functions. In this case, the lack of parens emphasizes the "path to nested data" pattern which is the heart of what's really going on. In Fennel this pattern cannot be implemented with -> because we don't have keyword functions, but we do have (. m :key :inside :table) syntax which accomplishes the same thing using a macro which is more specifically suited for this case. Because of this difference, IMO the rationale for making the parens optional is not as strong.

@noisesmith

This comment has been minimized.

Copy link
Contributor Author

commented Jan 2, 2019

If that's the prevailing view I'm OK with not merging this feature. We might want to look at the current behavior:

(cmd)>> (fn *2 [x] (* x 2))
#<function: 0x55a859f8af80>
(cmd)>> (-> 5 *2)
#<function: 0x55a859f8af80>
(ins)>> ((-> 5 *2))
[string "local _2a2 = ___replLocals___['_2a2']..."]:3: attempt to perform arithmetic on local 'x' (a nil value)
stack traceback:
  [string "local _2a2 = ___replLocals___['_2a2']..."]:3: in function ?
  /usr/local/share/lua/5.2/fennel.lua:1902: in function ?
  [C]: in function 'xpcall'
  /usr/local/share/lua/5.2/fennel.lua:1902: in function 'repl'
  /usr/local/bin/fennel:110: in main chunk
  [C]: in ?

if we don't want to coerce the forms to lists, perhaps we should explicitly error with an informative message when compiling if they aren't lists

@technomancy

This comment has been minimized.

Copy link
Collaborator

commented Jan 2, 2019

@technomancy technomancy merged commit e6cc3b1 into bakpakin:master Jan 3, 2019
1 check passed
1 check passed
ci/circleci Your tests passed on CircleCI!
Details
@technomancy

This comment has been minimized.

Copy link
Collaborator

commented Jan 3, 2019

Talked about it on the list and it sounds like we should get this in. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.