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

Handle empty items in arrays and arraylike objects #75

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

ninevra
Copy link
Contributor

@ninevra ninevra commented Jul 20, 2021

Adds an EmptyItem descriptor to represent the gaps in sparse arrays. Formats gaps as <empty item>, and treats them as unequal to anything other than empty items for comparison and diffing.

Adds theme keys

item: {
    empty: {
        open: '<',
        close: '>',
    }
}

for controlling the formatting; I'm not sure if this is the best way to do this. Maybe the item text should be configurable, or maybe the brackets should be part of the item text and not part of the open/close wrappers?

I initially implemented this with an Empty primitive descriptor, but I think EmptyItem better matches the data model. I can't think of a non-list context where Empty would be valid.

This PR does not collapse empty items into e.g. <5 empty items> like util.inspect does; that functionality seemed like a subset of #12.

Closes #74.

This is a breaking change & bumps the serialization format version to 4.

@ninevra
Copy link
Contributor Author

ninevra commented Jul 23, 2021

There's a bug in the alignment code that I need to figure out.

Copy link
Member

@novemberborn novemberborn left a comment

Choose a reason for hiding this comment

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

This is great!

Maybe the item text should be configurable, or maybe the brackets should be part of the item text and not part of the open/close wrappers?

Perhaps the entire <empty item> could be the theme string, and then tools like AVA can color it however they like?

I initially implemented this with an Empty primitive descriptor, but I think EmptyItem better matches the data model. I can't think of a non-list context where Empty would be valid.

👍

This PR does not collapse empty items into e.g. <5 empty items> like util.inspect does; that functionality seemed like a subset of #12.

👍

This is a breaking change & bumps the serialization format version to 4.

👍

Comment on lines +70 to +76
test('empty array slots do not equal undefined', t => {
t.false(compare(new Array(1), Array.from({ length: 1 })).pass)
})

test('empty arraylike slots do not equal undefined', t => {
t.false(compare({ length: 2, 0: 'a' }, { length: 2, 0: 'a', 1: undefined }).pass)
})
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps this needs some tests for equality between arrays with empty items?

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.

Sparse arrays are treated like arrays of undefined
2 participants