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

parser key grouping - question #1206

Open
petschki opened this issue Oct 3, 2024 · 1 comment
Open

parser key grouping - question #1206

petschki opened this issue Oct 3, 2024 · 1 comment
Assignees

Comments

@petschki
Copy link
Member

petschki commented Oct 3, 2024

We have a pattern with the following arguments:

parser.addArgument("recently-used");
parser.addArgument("recently-used-key");
parser.addArgument("recently-used-max-items", 20);

with the following markup

<div class="pat-contentbrowser" data-pat-contentbrowser='{
    "recentlyUsed": true,
    "recentlyUsedKey": "my_field_key"
    "recentlyUsedMaxItems": 10
}'></div>

what I expect:

you get the parsed options like this:

{
    "recentlyUsed": true,
    "recentlyUsedKey": "my_field_key",
    "recentlyUsedMaxItems": 10
}

what you get

there's the new "group" key recently which holds the default value:

{
    "recentlyUsed": true,
    "recentlyUsedKey": "my_field_key",
    "recentlyUsedKeyMaxItems": 10,
    "recently": {
        "used-max-items": 20
    }
}

now this gets a bit confusing because If we remove an option from the markup (for example the recentlyUsedMaxItems), I expect the this.options.recentlyUsedMaxItems contains the default value from the parser but its missing completely in the dict:

{
    "recentlyUsed": true,
    "recentlyUsedKey": "my_field_key",
    "recently": {
        "used-max-items": 20
    }
}

Only the grouped recently key contains this value.

Further if you remove the default value in parser.addArgument there is no more grouping of the keys at all.

Am I missing some logic here, or is this a bug?

@thet
Copy link
Member

thet commented Oct 3, 2024

There is a smartypants grouping functionality since long, which is confusing, I agree.
The grouping depends on the defined arguments via parser.addArgument, not the actual arguments used in the markup.

So, there was no change when removing the argument from the markup (good) but there was a change when removing the argument from the argument definition (bad for consistency).

I'd agree to see this even as a bug and this should be fixed - maybe with an option which always normalizes the argument list to camel case without any grouping.

But there is already something similar for new-style base class based patterns: If you define parser_group_options in the Patterns base class, no grouping, but also no camelCasing will take place. Instead you'd access the arguments exactly the way you had defined them. In this case this.options["recently-used-key"]. I'd use this instead of the automatic but unstable grouping.

See: https://github.com/Patternslib/Patterns/blob/master/src/core/basepattern.js#L20
https://github.com/Patternslib/Patterns/blob/master/src/core/parser.js#L377

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

2 participants