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

Make sure records in parenthesis are treated the same #674

Merged
merged 3 commits into from
Dec 9, 2016
Merged

Conversation

ibdknox
Copy link
Contributor

@ibdknox ibdknox commented Dec 9, 2016

In code where sub-records were defined in a parenthesis, we weren't pushing the parent's identity down into the children, we also weren't setting eve-auto-index on them. This makes sure we do both and should remove any differences between using parens or not.

Fixes. #673

Copy link
Contributor

@convolvatron convolvatron left a comment

Choose a reason for hiding this comment

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

couldn't find anything wrong here...tried to track back the item.from chain, but got lost

just aside, do we have any concrete plans to shoot eve-auto-index?

@ibdknox
Copy link
Contributor Author

ibdknox commented Dec 9, 2016

eve-auto-index serves a pretty important pragmatic purpose, but maybe there's some other way to accomplish what it gives us. The main value is in something like this:

bind @browser
  [#div children: 
     [#div text: "foo"]
     [#div text: "bar"]
     [#div text: "baz"]]

Since eve-auto-index is applied to these, we know to keep them in that order on display. If we didn't have something like that you'd need to supply sort on all of them despite the fact that we've already expressed the intent.

@ibdknox
Copy link
Contributor Author

ibdknox commented Dec 9, 2016

Where that overhead would really get you is if you were inserting something in the middle and then had to update every sort attribute underneath. Do that a couple times and it'll seem worse than busy work.

@ibdknox ibdknox merged commit f36a33f into master Dec 9, 2016
@ibdknox ibdknox deleted the fix-673 branch December 12, 2016 22:11
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.

2 participants