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

Add docs and tests for as-> macro #1141

Merged
merged 5 commits into from
Nov 3, 2016
Merged

Add docs and tests for as-> macro #1141

merged 5 commits into from
Nov 3, 2016

Conversation

tuturto
Copy link
Contributor

@tuturto tuturto commented Oct 25, 2016

Adds docs and tets for as-> macro. I had trouble running tests locally, but now that I tried to reproduce it to write down the error message they passed. So please check if that happens on your machine too.

relates to #1047


.. note::

In these examples, REPL will report a tupple as result:
Copy link
Contributor

Choose a reason for hiding this comment

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

tuple? (Also, missing the before REPL.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, tuple is one of those words that I consistently misspell.

The previous result is thus available in the subsequent form. Returns the final
result, and leaves the name bound to it in the local scope. This behaves much
like the other threading macros, but requires you to specify the threading
point per form via the name instead of always the first or last arument.
Copy link
Contributor

Choose a reason for hiding this comment

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

...this is kind of confusing. I mean, I'm guessing that there will likely never be a "simple" explanation for something like this. I think maybe at minimum the example should use something more descriptive than x; maybe assigned_name?

Copy link
Contributor Author

@tuturto tuturto Oct 25, 2016

Choose a reason for hiding this comment

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

true, assigned-name is much better. I'll try to think better explanation, but currently I'm drawing a blank.

Copy link
Member

@gilch gilch Oct 27, 2016

Choose a reason for hiding this comment

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

I disagree that assigned-name is better. Perhaps you could use it in the first example just to point out which argument is which, but certainly not for all of them. The whole point of as-> is an alternative to -> and ->>, but where the insertion point can be anywhere in the list, instead of the first or last argument only. For deeply nested calls, this name may have to be repeated many times. Therefore, idiomatic usage is with a very short name, perhaps one or two letters. (This is not a problem for -> and ->> where the point is implied by the position.)

Perhaps the documentation would be clearer if you demonstrated how to accomplish a -> using as->. You can also look at Clojure's community documentation for clarification.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of assigned-name I could also us it as it's short and generic enough. I'll try to come up with a better examples still, that would highlight usage of as-> better (as now the example could be easily rewritten with ->).

.. note::

In these examples, REPL will report a tupple as result:
('Sepia prashadi', 'Sepia prashadi'), but the actual value returned is just
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be surrounded by double-backticks for formatting? The sentence could also be worded a bit differently, like:

In these examples, the REPL will report a tuple (e.g. `('Sepia prashadi', 'Sepia prashadi')`) as the result, but only a single value is actually returned.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll update this

@tuturto tuturto added this to the 0.12.0 milestone Oct 26, 2016
@gilch
Copy link
Member

gilch commented Oct 27, 2016

This might be a good time to fix that typo in the last word of the as-> docstring (or even rewrite the whole thing to make it clearer). Not that we can even access those for macros #892.

@tuturto
Copy link
Contributor Author

tuturto commented Oct 27, 2016

Good catch with the typo @gilch. I hadn't fixed it in both locations.

@tuturto
Copy link
Contributor Author

tuturto commented Oct 29, 2016

Polished the documentation a bit again. Anything else that needs some cleaning up here or would this soon be good to go?

@olasd
Copy link
Member

olasd commented Nov 2, 2016

👍

1 similar comment
@refi64
Copy link
Contributor

refi64 commented Nov 2, 2016

👍

@tuturto
Copy link
Contributor Author

tuturto commented Nov 3, 2016

As this has multiple thumbs up already, I'll merge this one too (even if I'm the originator).

@tuturto tuturto merged commit f60ed24 into hylang:master Nov 3, 2016
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