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

Allow dynamic array indexing in a data address. #547

Closed
wants to merge 1 commit into from

Conversation

exjam
Copy link
Contributor

@exjam exjam commented Dec 4, 2023

This change allows arrays to be dynamically indexed e.g. data-checked="views[view_index].filter.chat" is now valid whereas previously in place of view_index you could only use an integer literal.

This seems to work though I am not sure if there are any complications that I do not know about given that I am quite new to RmlUI.

If you're happy with the approach I have taken here, I could add some unit tests to cover this use case.

@exjam exjam force-pushed the dynamic_array_index branch from f2b34d9 to 88c4662 Compare December 4, 2023 14:55
@exjam exjam force-pushed the dynamic_array_index branch from 88c4662 to 672ee78 Compare December 4, 2023 15:10
@mikke89 mikke89 added enhancement New feature or request data binding labels Dec 6, 2023
@mikke89
Copy link
Owner

mikke89 commented Dec 6, 2023

Very cool, this has been something that's been requested several times by users. It would be a very nice addition.

I wonder if it is possible to do this purely through our data expression language. It would be really cool if we could use arbitrary data expressions to index into the arrays. E.g. view_index / 2 + 3. This should come pretty much for free if we can implement this functionality within our parser. What do you think about this, is this something you'd want to investigate?

It would be nice with some unit tests for this, you can take a look at UnitTests/DataBinding.cpp or UnitTests/DataExpression.cpp and continue from there.

@Dakror I believe you started looking into this feature. Maybe you want to help review and assist on this one?

@Dakror
Copy link
Contributor

Dakror commented Dec 7, 2023

Thats what i get for not posting my PR. I have a complete dynamic address expression parsing branch lying around on my project.

I'll see how or if we can combine these two branches.

@exjam
Copy link
Contributor Author

exjam commented Dec 7, 2023

I would appreciate if you could share your code, I do not see an immediately obvious way to expand this to full expression parsing given that I'd need to find some way to plumb an expression interface through to the Child() call

@Dakror
Copy link
Contributor

Dakror commented Dec 7, 2023

The answer is that there is no obvious way to do that at all but instead you need to break open large parts of the interpreter. I've submitted my code in #550 for you to look at. This way we can decide what to keep now that there seems some more interest in this functionality.

@exjam exjam force-pushed the dynamic_array_index branch from f94dfc0 to 672ee78 Compare January 29, 2024 16:39
@exjam
Copy link
Contributor Author

exjam commented Jan 29, 2024

Thanks @Dakror , your PR #550 works great for me, so will close this.

Sorry for disappearing for so long, my day job got very busy and I had no time to work on this side project!

@exjam exjam closed this Jan 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data binding enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants