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

Added x and xx example, fixes #278. Also fixes tutorial. #286

Merged
merged 7 commits into from
May 8, 2018

Conversation

dan-simon
Copy link
Contributor

This PR adds an x and xx example (removing the hello world example). It also updates the tutorial's lists of builtin functions and methods (though it does not include regex or type methods). (The reason I updated the tutorial was that I didn't know about the concat or push list methods and so initially wrote a big mess. I don't want anyone else making the same mistake.)

Potential issues:

  • I implemented xx to flatten list input. So 1 xx 3 is [1, 1, 1] and [1, 2] xx 3 is [1, 2, 1, 2, 1, 2]. Is this the desired behavior?
  • I don't know how to run all the tests. All I did is create a new test file and run that, while deleting the hello world test file. Are there any other style tests or such I should run?

@dan-simon
Copy link
Contributor Author

This was meant to be "fixes #278", not "fixes #287", of course.

@masak masak changed the title Added x and xx example, fixes #287. Also fixes tutorial. Added x and xx example, fixes #278. Also fixes tutorial. May 8, 2018
@masak
Copy link
Owner

masak commented May 8, 2018

I just discovered this PR. Good work! I will comment on lines of code individually, but what I see already looks good enough that I'd like to merge ASAP.

Re your two questions:

  • I hesitated in many different directions with respect to flattening. Even had to go back to see how the old, removed infix:<xx> worked. (It flattens.) For now we'll let the new one flatten too.
  • The proper incantation for running the test suite is prove -e'perl6 -Ilib' -r t/. (I'll consider putting it somewhere in the vicinity of the README file.) I have an alias set up locally which runs the prove command after first running an empty program in bin/007 (because this compiles all the module files first before running the test suite, which avoids getting those errors through the test harness).

sub flatten (inputList) {
my result = [];
for inputList -> i {
if (type(i) == type([])) {
Copy link
Owner

Choose a reason for hiding this comment

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

This works, but more idiomatic to use type matching: if i ~~ Array.

if (type(i) == type([])) {
for i -> j {
result.push(j);
}
Copy link
Owner

Choose a reason for hiding this comment

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

Shorter and no less clear than the for loop might be to do result = result.concat(i); — this appends all the elements "at once" at the expense of creating a brand new array and letting the old one be collected.

return quasi {
flatten((^{{{right}}}).map(sub getValue(ignore) {
return {{{left}}};
}))
Copy link
Owner

Choose a reason for hiding this comment

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

This is beautiful. ❤️


# Test (list or number) xx number.
my j = 0;
say((j = [1, [1, 2, 3]][j && 1]) xx 2);
Copy link
Owner

Choose a reason for hiding this comment

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

No worries for now, but given the way #279 is heading, this expression might become illegal soon. Just a heads-up. 😄 It's an interesting way to check for something — if we outlaw assignment expressions, we'll have to make sure to have an alternative in place for this one.

@masak
Copy link
Owner

masak commented May 8, 2018

All I made was minor, non-merge-blocking comments, so I'm going to go ahead and merge this now. Again, great work, especially for a first contribution!

@masak masak merged commit a8f6471 into masak:master May 8, 2018
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.

None yet

2 participants