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

feat(table): improve pretty-printing for simple tables and lists #478

Merged
merged 1 commit into from
Nov 9, 2024

Conversation

vhyrro
Copy link
Contributor

@vhyrro vhyrro commented Nov 2, 2024

Hi,

This PR improves the pretty printing code for Lua tables:

  • Adds support for list rendering
  • Adds support for rendering simple keys (["hello"] = 3) as just hello = 3

The use case for this is reading in a Lua file (like a rockspec), modifying its globals and then reconstructing the Lua file in a readable format. Given that debug pretty printing is for debugging (duh), I'm not classifying this as any sort of breaking change :)

One note: the is_sequence check goes through all keys in a table and checks if they're integers and in ascending order. The is_list() method is only for cfg!(serialize), so I've refrained from refactoring that in fear of breaking some obscure behaviour. Let me know if there's anything you'd change!

@vhyrro
Copy link
Contributor Author

vhyrro commented Nov 5, 2024

Saw that luau is failing, on it :)

edit: done! There's one test failure that's not caused by this pull request.

@khvzak khvzak merged commit 0fda512 into mlua-rs:main Nov 9, 2024
136 checks passed
@khvzak
Copy link
Member

khvzak commented Nov 9, 2024

Thanks

@khvzak
Copy link
Member

khvzak commented Nov 9, 2024

I just noticed an edge case where tables like {["1"] = "first", ["2"] = "second"} will be wrongly formatted as:

{
  1 = "first",
  2 = "second",
}

This table cannot be loaded back to Lua.

Fixed in b34b90e

@vhyrro
Copy link
Contributor Author

vhyrro commented Nov 13, 2024

Oh good catch, sorry for that :)

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