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

Support short array syntax matching short object notation of ES2015 #336

Open
albe opened this issue Aug 10, 2017 · 13 comments
Open

Support short array syntax matching short object notation of ES2015 #336

albe opened this issue Aug 10, 2017 · 13 comments

Comments

@albe
Copy link
Contributor

albe commented Aug 10, 2017

There are a lot of use-cases where one must pass on a number of template variables to a partial, in which case one often ends up with something like the following:

   <f:render partial="Foo" arguments="{ something: something, somethingElse: somethingElse, somethingWholeDifferent: somethingWholeDifferent, andAlsoThis: WhichIsSomethingElse }">

It would be cool if Fluid supported a short array notation like the short object notation of ES2015 (https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Operators/Object_Initializer#New_notations_in_ECMAScript_2015).

The above would then become:

   <f:render partial="Foo" arguments="{ something, somethingElse, somethingWholeDifferent, andAlsoThis: WhichIsSomethingElse }">

Edit: There is currently one big issue with this feature: It makes the expression { foo } ambiguous to mean "the value of the variable 'foo'" or "an array with key 'foo' and value of the variable 'foo'". Any ideas on how to approach this are welcome.

@albe
Copy link
Contributor Author

albe commented Aug 10, 2017

Something like this should do for parser regexes:

$SCAN_PATTERN_SHORTHANDSYNTAX_ARRAYS = '/^
    (?P<Recursion>                                             # Start the recursive part of the regular expression - describing the array syntax
        {                                                      # Each array needs to start with {
            (?P<Array>                                         # Start sub-match
                (?:
                    \s*(?:
                        (
                            [a-zA-Z0-9\\-_]+                       # Unquoted key
                            |"(?:\\\"|[^"])+"                      # Double quoted key, supporting more characters like dots and square brackets
                            |\'(?:\\\\\'|[^\'])+\'                 # Single quoted key, supporting more characters like dots and square brackets
                        )                                          # END possible key options
                        \s*:\s*                                    # Key|Value delimiter :
                        (?:                                        # Possible value options:
                            "(?:\\\"|[^"])*"                       # Double quoted string
                            |\'(?:\\\\\'|[^\'])*\'                 # Single quoted string
                            |[a-zA-Z0-9\-_.]+                      # variable identifiers
                            |(?P>Recursion)                        # Another sub-array
                        )                                          # END possible value options
                    )                                          # END key-value syntax
                    |[a-zA-Z0-9\\-_]+                          # Single unquoted key matching short array syntax
                    \s*,?                                      # There might be a , to separate different parts of the array
                )*                                             # The above cycle is repeated for all array elements
            )                                                  # End array sub-match
        }                                                      # Each array ends with }
    )$/x';

$SPLIT_PATTERN_SHORTHANDSYNTAX_ARRAY_PARTS = '/
    (?P<ArrayPart>
        (?:                                                                 # Start sub-match of one key and value pair
            (?P<Key>                                                        # The arry key
                 [a-zA-Z0-9_-]+                                             # Unquoted
                |"(?:\\\\"|[^"])+"                                          # Double quoted
                |\'(?:\\\\\'|[^\'])+\'                                      # Single quoted
            )
            \\s*:\\s*                                                       # Key|Value delimiter :
            (?:                                                             # BEGIN Possible value options
                (?P<QuotedString>                                           # Quoted string
                     "(?:\\\\"|[^"])*"
                    |\'(?:\\\\\'|[^\'])*\'
                )
                |(?P<VariableIdentifier>
                    (?:(?=[^,{}\.]*[a-zA-Z])[a-zA-Z0-9_-]*)                 # variable identifiers must contain letters (otherwise they are hardcoded numbers)
                    (?:\\.[a-zA-Z0-9_-]+)*                                  # but in sub key access only numbers are fine (foo.55)
                )
                |(?P<Number>[0-9]+(?:\\.[0-9]+)?)                           # A hardcoded Number (also possibly with decimals)
                |\\{\\s*(?P<Subarray>(?:(?P>ArrayPart)\\s*,?\\s*)+)\\s*\\}  # Another sub-array
            )                                                               # END possible value options
        )
        |(?P<KeyValue>[a-zA-Z0-9_-]+)                                       # The arry key and value in short syntax
    )\\s*(?=\\z|,|\\})                                                  # An array part sub-match ends with either a comma, a closing curly bracket or end of string
/x';

The recursiveArrayHandler would then just add a check early in the loop like this:

	if (!empty($singleMatch['KeyValue'])) {
		$arrayToBuild[$singleMatch['KeyValue']] = new ObjectAccessorNode($singleMatch['KeyValue']);
		continue;
	}

If this agreed on, I would take care to provide the PR with the necessary changes and tests.

@kdambekalns
Copy link
Contributor

In the first example, how is the matching to argument names done? Not at all, so order must be correct? The beauty of the current way of passing arguments is the support for named arguments, something not used in that example. Or is it merely a bad example?!

For short array syntax as such: 👍
For use instead of named arguments: 👎

@bwaidelich
Copy link

If I get it right, the argument name would be equal to the variable name, so these two would be equivalent:

{ foo: foo, bar: bar }
{ foo, bar }

One possible caveat might be the ambiguity between the regular object accessor syntax and a "shorthand array" with one item ({foo})

@albe
Copy link
Contributor Author

albe commented Aug 11, 2017

Yes, exactly that @bwaidelich and good point with the object accessor syntax, that could be an issue. Not sure how to best deal with that in fact. At least for cases where { foo } evaluates to a non-array, that could/would be solved via type checks, but that still leaves the case when 'foo' is an array variable. Maybe just document it clearly that short array syntax only works with > 1 elements and cross fingers?

@kdambekalns not removing/replacing named arguments (or anything else). Just providing an additional shorter notation for 1:1 mapping of variable name to array key. It should be totally free to mix with the current notation per attribute.

Edit: To be more clear, this should be totally fine:

{ foo, bar: bar, baz: bazinga, quux: { hido, hidi: '{f:translate(...)}`, whoop: '{say.what}' } }

@kdambekalns
Copy link
Contributor

Ok, so the variable name is used as key. I wasn't sure the name of the passed variable is known, but a bit of thinking would have cleared that up (we are looking at parsing the code here, doh!)

So, well, great. I can imagine shooting myself in the foot with this, but it'll be a pleasure, most of the time… ;)

@sascha-egerer
Copy link
Contributor

@albe You can also do something like this:

    <f:alias map="{andAlsoThis: WhichIsSomethingElse}">
       <f:render partial="Foo" arguments="{_all}">
    </f:alias>

That will pass all variables to the partial but you can also define your own variables via the wrapping alias.

@bwaidelich
Copy link

I try to avoid using _all in my Fluid Templates (it's prefixed for a reason) ;)
While it's very convenient in times it makes the partials less reusable if you pass them the whole world as context.

@sascha-egerer
Copy link
Contributor

sascha-egerer commented Aug 11, 2017

Yes but if you need to pass all variables to the partial anyway but do also need some custom one you can do it like that.

@NamelessCoder
Copy link
Contributor

Couple of things come to my mind:

  • I would not object to detecting arrays this way as well because I know the use case all too well.
  • I'd also like to point out that you could actually achieve this right now without manipulating the parsing expressions, through means of an ExpressionNode implementation that detects CSV variable references and creates an associative array from that.
  • A solution to this needs to be crafted quite carefully because inevitably, people will begin to guess and use things like {1, 2, 3} or {'one', 'two'} which we may not necessarily want to support (since it adds to the number of ways you could write invalid array syntax). If we do support those, we'd need to decide on the behavior (should value be used as key if strings are passed? If ints are passed? What about booleans? And so on).
  • Assuming we will only ever allow this when actually detecting arrays, we can allow creating an associative array from a single element as well, but this may be too confusing to the user (same syntax passes an array in one case and the value itself in another case).

Just some things to be careful about :)

@NamelessCoder
Copy link
Contributor

Btw, thinking out loud regarding _all we could replace that with *:

  • <f:render partial="Test" arguments="*" />
  • {f:render(partial: 'Test', arguments: *)}

...and make it exclusively a different way of detecting arrays. This would also potentially improve performance because the special _all variable name no longer needs to be checked every time an ObjectAccessor is asked for a value, which also improves the compiled templates. The array argument's value can then be extracted using $renderingContext->getVariableProvider()->toArray() which also fits nicely into a compiled template.

@albe
Copy link
Contributor Author

albe commented Aug 11, 2017

@sascha-egerer Yes, but the use case is really where you only want to pass a sub set of the variables, which happens quite often (at least for me). Also I prefer to be explicit about what I do and passing "_all" is very implicit in the sense that you need to know what "_all" is in the current context. So if I for example need to change some partial code and want to know which variables are available (esp. when I didn't write the template/partial in the first place), I prefer to only have to search one level up to the caller to see what I have at hand, rather than go n levels up and eventually land in the controller.

@NamelessCoder I would totally not allow { 1, 2, 3 } or { 'a', 'b', "c" } - that's also not allowed in ES2015. For something like defining a non-associative array I'd rather introduce an specific syntax for that [ 1, 2, 3 ] instead. But that's beyond the scope of this.
Thanks for the hint at the " ExpressionNode implementation that detects CSV", I'll have a thought about that :)
The last point with the ambiguity is the most concerning to me actually, and I have no good idea how to solve that :/

@albe
Copy link
Contributor Author

albe commented Aug 16, 2017

Ok, so after thinking a bit about the ExpressionNode, I think this would not work easily with mixed notation, e.g. { a, b, c: d, e: { f, g: 'h' } }. At least not unless the ExpressionNode would copy a lot of the Array Syntax Regex (and logic). So I'm in favor of just adding the shorthand pattern as suggested above.

@masi
Copy link

masi commented May 15, 2023

It's time that Fluid becomes more user oriented. Vue templates are much more fun to write.

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

No branches or pull requests

6 participants