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

Lune crashing when printing cyclic table #156

Closed
SnorlaxAssist opened this issue Feb 22, 2024 · 9 comments · Fixed by #158
Closed

Lune crashing when printing cyclic table #156

SnorlaxAssist opened this issue Feb 22, 2024 · 9 comments · Fixed by #158
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@SnorlaxAssist
Copy link
Contributor

I notice when printing a table of its own, has a triple ..., I assume to refer as a cyclic table reference.
eg:

local t = {}
t[1] = t
print(t)
{
    [1] = {
        [1] = {
            [1] = {
                [1] = { ... },
            },
        },
    },
}

but when the table is on the key itself, it would crash lune.
eg:

local t = {}
t[t] = 1
print(t)
thread 'main' has overflowed its stack

Perhaps instead of printing the table within a key index, something like how Roblox handles it, which is just the address of the table.
{ [Table(1B163F80D68)] = 1 }

@filiptibell filiptibell added bug Something isn't working good first issue Good for newcomers labels Feb 22, 2024
@filiptibell
Copy link
Collaborator

I was wondering when this would get reported 😅 we have had a TODO comment here since what feels like forever:

// TODO: Handle tables with cyclic references

If anyone wants to PR in a fix for this, feel free!

@SnorlaxAssist
Copy link
Contributor Author

I have a temporary Lua Solution to this, that has the same color layout as Lune's built-in print.

local stdio = require("@lune/stdio")

local Formatter = {}

local MAX_FORMAT_DEPTH = 4
local INDENT = "    "

local COLOR_CYAN = stdio.color("cyan")
local COLOR_BLUE = stdio.color("blue")
local COLOR_YELLOW = stdio.color("yellow")
local COLOR_GREEN = stdio.color("green")
local COLOR_PURPLE = stdio.color("purple")
local COLOR_RESET = stdio.color("reset")

local STYLE_DIM = stdio.style("dim")
local STYLE_RESET = stdio.style("reset")

local function applyStyle(style : string, str : string)
	return `{style}{str}{STYLE_RESET}`
end

local function applyColor(color : string, str : string)
	return `{color}{str}{COLOR_RESET}`
end

function Formatter.formatValue(value : any, depth : number, asKey : boolean?)
	depth = depth or 0

	if depth >= MAX_FORMAT_DEPTH  then
		return applyStyle(STYLE_DIM, "{ ... }")
	end

	if value == nil then
		return "nil"
	elseif type(value) == "boolean" then
		return applyColor(COLOR_YELLOW, tostring(value))
	elseif type(value) == "number" then
		return applyColor(COLOR_CYAN, tostring(value))
	elseif type(value) == "string" then
		return `"{applyColor(COLOR_GREEN, (value:gsub(`[%z\128-\255]`, ""):gsub('"', '\\"'):gsub('\n', '\\n')))}"`
	elseif type(value) == "function" then
		return applyColor(COLOR_PURPLE,`<{tostring(value)}>`)
	elseif type(value) == "table" then
		if asKey then
			return applyColor(COLOR_BLUE, `<{tostring(value)}>`)
		end
		local result = {}
		for k, v in value do
			if type(k) == "string" then
				table.insert(result, `\n{string.rep(INDENT, depth + 1)}{k} {applyStyle(STYLE_DIM, '=')} {Formatter.formatValue(v, depth + 1)}`)
			else
				table.insert(result, `\n{string.rep(INDENT, depth + 1)}[{Formatter.formatValue(k, depth, true)}] {applyStyle(STYLE_DIM, '=')} {Formatter.formatValue(v, depth + 1)}`)
			end
		end
		if #result == 0 then
			return applyStyle(STYLE_DIM, "{}")
		end
		table.insert(result, `\n{string.rep(INDENT, depth)}{applyStyle(STYLE_DIM, '}')}`)
		return `{applyStyle(STYLE_DIM, '{')}{table.concat(result, `{applyStyle(STYLE_DIM, ',')}`)}`
	else
		return `<{tostring(value)}>`
	end
end

function Formatter.print(... : any)
	local args = {...}
	local formattedArgs = {}
	for i, v in args do
		if type(v) == "string" then
			table.insert(formattedArgs, v)
		else
			table.insert(formattedArgs, Formatter.formatValue(v, 0))
		end
	end
	stdio.write(`{table.concat(formattedArgs, "  ")}\n`)
end

return Formatter;

A side by side comparison
image

@CompeyDev
Copy link
Contributor

If anyone wants to PR in a fix for this, feel free!

I'll PR this over tonight, in that case!

@CompeyDev
Copy link
Contributor

CompeyDev commented Feb 24, 2024

I've fixed this locally, will create the PR later today.

EDIT: We need to decide on one thing though, what should be displayed for formatting cyclic tables? I was thinking of something like <self>, over the address to the table (that roblox does) or { ... } (which lune currently uses to truncate tables past max depth).

CC @filiptibell @SnorlaxAssist

@SnorlaxAssist
Copy link
Contributor Author

SnorlaxAssist commented Feb 24, 2024

<self> could be confusing as it can be interpreted as another data type, maybe just <table>?

image
(examples of how some datatypes are shown in lune as keys)

@CompeyDev
Copy link
Contributor

<self> could be confusing as it can be interpreted as another data type, maybe just <table>?

Well no, if we want to follow what lune already does, it would make more sense to use { ... }. The main reason I wanted it to be <self> was because it clarifies that it is a cyclic reference, and not just a generic truncation.

@SnorlaxAssist
Copy link
Contributor Author

I understand the concept of self, but as it is used in <self> could be confusing to anyone not understanding it's not a native luau type, such that <userdata> is userdata datatype, <function> is function datatype, etc. On the other hand { ... } would actually be the better option, as it signifies cyclic reference to itself, whilst remaining consistent to showing tables.

@CompeyDev
Copy link
Contributor

CompeyDev commented Feb 26, 2024

Well, that doesn't exactly signify a cyclic reference, just that the table is being truncated.

I say we take the best of both worlds with something like { self }, or an address since that differenciates it from a regular truncation.

@SnorlaxAssist
Copy link
Contributor Author

SnorlaxAssist commented Feb 26, 2024

I mean it was one of your options, I assume { self } works too, and better since it is clearer, I would like to know what @filiptibell thinks about it.

Edit: Anything would work, as long as the issue is getting fixed, it is mostly an inconvenience with this bug, not a big deal but also could be hard to track because of the crash message being unclear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants