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

iteration and branching in named argument lists #807

Closed
gavinking opened this issue Sep 30, 2013 · 10 comments
Closed

iteration and branching in named argument lists #807

gavinking opened this issue Sep 30, 2013 · 10 comments

Comments

@gavinking
Copy link
Member

To my surprise there is no actual issue dealing with this topic, though it has been discussed episodically over the last 4 years, and even used to be the subject of a TODO in the spec, before it got "cleaned up" one day.

We already have limited support for doing branching and looping inside a named argument list, using:

  • comprehensions, and
  • then/else.

With enhancements already planned for 1.2, we will also get real conditional expressions.

For example:

Div {
    Div { Span("Items") },
    if (items.empty) then
        Div { 
            Span("no items") 
        }
    else
        Div {
            for (i->item in items.indexed)
                Div {
                    Span("``i``."),
                    Span(item.description)
                }
        }
    }
}

However, that's still not going to be quite enough to let us write really freeform stuff in the section of the argument list that accepts a list of arguments. When we get a chance to spend some time on this, we need to figure out exactly how to support more than just a single comprehension there, allowing a mix of:

  • conditionals, with
  • comprehensions and listed arguments.

For example:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        if (items.empty) {
            Span(""),
            Span("no items"),
            Hr()
        }
        else {
            for (i->item in items.indexed) {
                Span("``i``."),
                Span(item.description),
                Hr()
            }
        }
    }
}

The above is not possible today because you can't have multiple children hanging off a conditional expression or comprehension and because comprehensions cannot be mixed with other arguments.

It's easy to just say that the solution to this problem is to allow freeform for loops and if/switch conditionals in a named argument list, and interpret its "statements" as argument expressions. But saying it like that hides a whole lot of things that need to be thought through properly.

In particular, we need to understand how this syntax relates to comprehensions as they exist today. It seems to me that the basic difference is that a comprehension or if expression expects a single expression at the end of it whereas for templating it's often more convenient to have more than one thing produced by a branch of the conditional or iteration of the loop. Hence the difference between the two code examples above is the braces.

I'm also a bit confused as to what is the natural punctuation for this. Commas, or semis? It's not clear which would be more consistent. Commas, probably.

@gavinking
Copy link
Member Author

This issue is very related, but nevertheless orthogonal, to #749.

@gavinking
Copy link
Member Author

A solution to this issue would be to:

Then we would wind up with something like this:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        *if (items.empty) then {
            Span(""),
            Span("no items"),
            Hr()
        }
        else {
            for (i->item in items.indexed) *{
                Span("``i``."),
                Span(item.description),
                Hr()
            }
        }
    }
}

The problem with this are the nasty *s. Trying to figure out a way to eliminate them.

P.S. A related idea is to allow comprehensions that begin with an if, for example:

if (something) Expression()

Unlike if/then expressions as proposed by #503, this would produce 0 elements, instead of a null element when the condition is unsatisfied. Or perhaps we should just eliminate if/then and require the else when an if expression appears outside of a variadic context.

@gavinking
Copy link
Member Author

So, over on #860, I concluded that maybe a Fragment class is the easiest thing on there eyes. The nice thing about Fragment is that it works just as well with an if comprehension or expression:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        if (items.empty)
            Fragment {
                Span(""),
                Span("no items"),
                Hr()
            }
        else
            Fragment {
                for (i->item in items.indexed)
                    Fragment {
                        Span("``i``."),
                        Span(item.description),
                        Hr()
                    }
            }
        }
    }
}

This is probably better than the *s in unintuitive locations.

@gavinking
Copy link
Member Author

Y'know, if we consider the if/else as an if comprehension, then the following would be consistent and reasonable. And note that the *s appear in the exact same locations and Fragment appears above.

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        if (items.empty) *{
            Span(""),
            Span("no items"),
            Hr()
        }
        else *{
            for (i->item in items.indexed) *{
                Span("``i``."),
                Span(item.description),
                Hr()
            }
        }
    }
}

@gavinking
Copy link
Member Author

Well, accommodating else in the comprehension syntax might be a bit too much. I would have to think that through. But you could still write it like this:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        if (items.empty) *{
            Span(""),
            Span("no items"),
            Hr()
        },
        if (!items.empty)
            for (i->item in items.indexed) *{
                Span("``i``."),
                Span(item.description),
                Hr()
            }
    }
}

That's honestly not bad.

And all it requires is:

That's really a pretty minor list of changes...

@gavinking
Copy link
Member Author

Perhaps I prefer this variation:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        *if (items.empty) {
            Span(""),
            Span("no items"),
            Hr()
        },
        *if (!items.empty)
            for (i->item in items.indexed) {
                Span("``i``."),
                Span(item.description),
                Hr()
            }
    }
}

@luolong
Copy link
Member

luolong commented Dec 4, 2013

To be honest, syntax wise I would actually prefer this variant:

Div {
    Div { Span("Items") },
    Div {
        Hr(),
        *{ if (items.empty) {
            Span(""),
            Span("no items"),
            Hr()
        }},
        *{ if (!items.empty)
            for (i->item in items.indexed) {
                Span("``i``."),
                Span(item.description),
                Hr()
            }
        }
    }
}

@gavinking
Copy link
Member Author

Actually there is a Good Reason to prefer the upfront *: it means that if comprehensions, and if/then/else look more regular. Compare:

Div {
    *if (items.empty) {
        Span(""),
        Span("no items"),
        Hr()
    }
}

With:

Div {
    *if (items.empty) 
    then {
        Span("")
    }
    else {
        Span("no items"),
        Hr()
    }
}

Since if/then/else is not a comprehension, * would not make sense in front of the {.

@gavinking
Copy link
Member Author

@luolong * and { cancel each other out in a named arg list: you're creating an iterable from the comprehension elements, and then spreading it over the args of the Div. So it's a noop.

@gavinking
Copy link
Member Author

I'm going to close this issue, since the solution is now captured by #860, #869, and #868.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants