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

vim.iter with non-list tables (as a replacement of vim.tbl_flatten, etc.) #28777

Closed
wookayin opened this issue May 16, 2024 · 2 comments · Fixed by #28781
Closed

vim.iter with non-list tables (as a replacement of vim.tbl_flatten, etc.) #28777

wookayin opened this issue May 16, 2024 · 2 comments · Fixed by #28781
Labels
enhancement feature request lua stdlib
Milestone

Comments

@wookayin
Copy link
Member

wookayin commented May 16, 2024

This issue would be more like a discussion, only stating potential issues from the perspective of practical user experience without a clear solution yet.

TL;DR) vim.iter have some practical problems when dealing with non-list tables (or pseudo-lists that contains nil as element), we might want to improve for better usability.

Problem

vim.tbl_flatten has been deprecated since nvim 0.10+ in favor of vim.iter, but what is suggested --- vim.iter(...):flatten():totable() is NOT a exact drop-in replacement of vim.tbl_flatten.

A corner case is when the table is "not a list", e.g. containing a nil values:

-- The table {1, nil, 3} is not a valid list (i.e. `vim.islist` returns false).
-- Also equivalent to {[1] = 1, [3] = 3}

vim.iter({ 1, nil, 3 }):flatten():totable()
-- ERROR: flatten() requires a list-like table

vim.tbl_flatten({ 1, nil, 3 })
-- { 1, 3 }

... because vim.iter treats list-like tables and non-list-like tables (i.e. maps) differently:

vim.iter({ 1, nil, 3 }):totable()
-- { {1, 1}, {3, 3} }

vim.iter({ 1, 2, 3 }):totable()
-- { 1, 2, 3 }

This is due to the limitation of the Lua language that Lua table does not distiniguish map and list(array), so a valid list contains. While this is actually an intended, correct behavior, given how Lua works, for users and laypeople who are not very familiar with Lua the behavior can be confusing: because totable() returns a list of (key, value) tuple for non-list-like tables.

Remark: it was an undefined behavior of tbl_flatten()

Actually, the use of vim.tbl_flatten() for tables that are not list-like (e.g. including nil as a member) was never specified:

tbl_flatten({t})                                           *vim.tbl_flatten()*
    Creates a copy of a list-like table such that any nested tables are
    "unrolled" and appended to the result.

    Parameters:  
      • {t}  (table) List-like table

    Return:  
        (table) Flattened copy of the given list-like table

    See also:  
      • From https://github.com/premake/premake-core/blob/master/src/base/table.lua

However, it is worth noting that vim.tbl_flatten() is often slightly abused in the wild to filter nil values while doing flattening:

Note: the behavior of vim.tbl_flatten() on such 'lists with holes (nil)' is not very accurate (as of nvim 0.10.0); it's not a 100% correct replacement of "filter nil values and then flatten":

> vim.tbl_flatten { nil, 1, nil, 3 }
< { 1, 3 }
> vim.tbl_flatten { 1, nil, 3 }
< { 1 }   -- ?!??!, should be { 1, 3 }

> vim.tbl_filter(function(v) return v end, { 1, nil, 3 })
< { 1, 3 }  -- correct

Potential impact:

Starting from the 0.11-dev (nightly) versions users will experience a looooot of deprecation warnings on the deprecated APIs, vim.tbl_flatten, vim.tbl_islist, etc. There are actually a lot of plugins that are still using vim.tbl_flatten which was deprecated only very recently. So plugin authors should migrate to vim.iter-based solution (unless we make additional changes) and remove the use of deprecated APIs sooner than later, but the subtle difference can lead to (minor) bugs.

Possible solutions

Either:

  • We don't change anything. Using nil in a list is an anti-pattern ?!
  • Implement flatten() for non-list-like (dict-like) tables.
  • Add some new options to vim.iter API to make vim.iter(table):totable() return a "map" instead of a list of (key, value) tuples.
  • Allow vim.iter() to handle lists with holes as well
    fix(vim.iter): enable optimizations for arrays (lists with holes) #28781
  • ???

References

@wookayin wookayin added the enhancement feature request label May 16, 2024
@lewis6991 lewis6991 added the lua stdlib label May 16, 2024
@gpanders
Copy link
Member

We don't change anything. Using nil in a list is an anti-pattern ?!

It does clearly have some valid and useful use cases, as we can see. So probably something we should support.

Implement flatten() for non-list-like (dict-like) tables.

I am less sure that flatten, specifically, needs to be the interface for this though (not opposed to it, but maybe this should be a separate/independent operation? vim.tolist or something?)

Add some new options to vim.iter API to make vim.iter(table):totable() return a "map" instead of a list of (key, value) tuples.

I was just looking at the vim.iter implementation again and I am wondering if it might actually be possible to support lists with holes. Currently we do this to determine if we can use the list optimizations:

    -- O(n): scan the source table to decide if it is a list (consecutive integer indices 1…n).
    local count = 0
    for _ in pairs(src) do
      count = count + 1
      local v = src[count]
      if v == nil then
        return Iter.new(pairs(src))
      end
      t[count] = v
    end
    return ListIter.new(t)

But the list optimizations use a bunch of regular for loops, and we track the head and tail of the list ourselves (we don't rely on the builtin length operator). So we might be able to instead change the semantics of vim.iter to treat any list with only integer keys (not necessarily consecutive) as a list, in which case vim.iter(...):flatten() would work as expected.

@echasnovski
Copy link
Member

vim.iter(...):flatten():totable() is NOT a exact drop-in replacement of vim.tbl_flatten.

Adding it here as it seems to be the best available place.

As vim.tbl_flatten() flattens all nested levels into a plain list, the proper drop-in replacement is vim.iter(t):flatten(math.huge):totable(). So both cases for { 1, { 2 }, { { 3, 4 }, { { 5 }, 6 } } } should return { 1, 2, 3, 4, 5, 6 }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature request lua stdlib
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants