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

change xi macro to #% tag macro #1444

Merged
merged 4 commits into from
Oct 29, 2017
Merged

change xi macro to #% tag macro #1444

merged 4 commits into from
Oct 29, 2017

Conversation

gilch
Copy link
Member

@gilch gilch commented Oct 23, 2017

As discussed in #1439. This replaces xi with a more Clojure-like #% tag macro.

@ekaschalk
Copy link
Contributor

Do you have thoughts on whether to highlight the %i? I don't know how clojure does it.

@gilch gilch force-pushed the xi-tag branch 2 times, most recently from 1d22965 to e6f9074 Compare October 23, 2017 02:06
@gilch
Copy link
Member Author

gilch commented Oct 23, 2017

You mean, "Does clojure-mode?" It does, I checked. Should Hy? I'm less certain. This is not builtin syntax. This is not even a core macro.

But, if we conventionally used %-prefixed anaphors in other macros (as Clojure does), then highlighting such symbols would make sense. It does improve readability. But some might argue the old xi symbols were more readable, because that's what won out in #879, even though I presented % symbols as an option.

We could use #xi with the old xi symbols. Maybe it's more readable, but it's also more likely to interfere with user symbols. Names like x1, x2 could totally come up. Tradeoffs.

@ekaschalk
Copy link
Contributor

I meant "does and what face", I can check into it.

A middle-ground is to implement is as an option, your call whether to enable it by default or not.

@gilch
Copy link
Member Author

gilch commented Oct 23, 2017

I added a %** anaphor for the &kwargs parameter. This makes #% usable for partial application even if the function takes keyword arguments. I also changed the &rest parameter from Clojure's %& to %* for consistency.

For example, #%(foo #* %* #** %**) would do the same thing as function foo, regardless of its signature.

@Kodiologist
Copy link
Member

This is cool. I like it. Is it ready for review? Still, I would finish up your outstanding PRs (and review those of others) before starting new ones like this.

@gilch
Copy link
Member Author

gilch commented Oct 26, 2017

Hy can't be my top priority all the time. I'm busy with other things right now, but this seemed like a really easy fix, especially since I wrote the code in question in the first place.

It's a tag macro, so I've generalized #% to work on arbitrary expressions.
For example, you can just use #%[%1 %2] instead of (xi identity [x1 x2]) like before.

Clojure would have to use #(identity [%1 %2]) or #(vector %1 %2) for that, but it saves the extra % in the more common case of a normal s-expression. We'd have to build function literals into the compiler to do it Clojure's way.

It should be ready for review now.

@Kodiologist
Copy link
Member

Kodiologist commented Oct 26, 2017

A lot of the diff is whitespace changes to old code. If you want to include them in this PR, could you put them in a separate commit?

Instead of "presence of a % parameter", I would write "presence of symbols named %*, %**, or %i for some number i".

The tests seem to pass without the (import hy) in anaphoric.hy, despite the use of HySymbol. So I think you can remove it.

(if (coll? expr) (flatten expr) (, expr)) can be written (flatten [expr]).

You have a commented-out require at the bottom of anaphoric.hy. I'm guessing it's old debugging code.

Otherwise, all looks good.

@gilch gilch force-pushed the xi-tag branch 2 times, most recently from 3a82b3a to 3b9895f Compare October 26, 2017 19:40
@gilch
Copy link
Member Author

gilch commented Oct 26, 2017

whitespace

Done.

Instead of "presence

I reworded it, but not quite that way.

(import hy)

Yep.

(flatten [expr])

More elegant, I like it.

old debugging code

Removed.

@Kodiologist, how about now?

=> (add-10 6)
16

``#%`` determines the parameter list by the presence of a ``%*``, or ``%**``
Copy link
Member

Choose a reason for hiding this comment

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

Stray comma.

@gilch gilch requested a review from tuturto October 26, 2017 20:31
@tuturto tuturto merged commit fac87c9 into hylang:master Oct 29, 2017
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.

4 participants