Skip to content

Handle empty items in arrays and arraylike objects#75

Open
ninevra wants to merge 11 commits intoconcordancejs:mainfrom
ninevra:empty-array-items
Open

Handle empty items in arrays and arraylike objects#75
ninevra wants to merge 11 commits intoconcordancejs:mainfrom
ninevra:empty-array-items

Conversation

@ninevra
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

ninevra commented Jul 23, 2021

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

Copy link
Copy Markdown
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
Copy Markdown
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