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

Make lua.lua display all tables #437

Open
wants to merge 2 commits into
base: master
from
Open

Conversation

@JakobDev
Copy link
Contributor

@JakobDev JakobDev commented Sep 7, 2017

At the moment, lua.lua can only display who are work with textutils.serialise(). With this PR, all table will be displayed.

Here's a screen how it looks, if textutils.serialise() not working:
cclite_1504794266

At the moment, lua.lua can only display who are work with textutils.serialise(). With this PR, all table will be displayed.
@SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Sep 7, 2017

I've been meaning to PR something like this for a while, so 👍 for getting on with it :). A couple of thoughts though:

  • IMO it'd be nicer to print the result of tostring instead of just the type. This means you can tell when two functions/coroutines/etc... are the same.
  • Ideally the above table would have the array-part entries be continuously, without their key. For instance, in your example you'd have: { "Hello", function, bol = true, tab = table, num = 1 }. This is the current behaviour of textutils.serialize.
  • It might be nice to use __tostring on child tables too, like done here.
  • This will fail on recursive structures, for instance _G. If one encounters a node you've already hit, then you should probably just use tostring( tDisplay ).
@JakobDev
Copy link
Contributor Author

@JakobDev JakobDev commented Sep 7, 2017

@SquidDev You mean, if __tostring is a function in a child table then call it and print the result? btw. it fails not with _G.

@SquidDev
Copy link
Contributor

@SquidDev SquidDev commented Sep 7, 2017

@Wilma456 No, it should look in the metatable for the __tostring implementation. See the linked code on how to implement that.

Ahh, you're not even recursing over nested tables, so you won't get any issues. Ideally you'd recur until you find something you've hit already, like textutils.serialize does. In fact, I'd use that as the basis for the implementation, and build from there.

@Wojbie
Copy link
Contributor

@Wojbie Wojbie commented Sep 7, 2017

Is this a good idea to not show contents of nested tables?

@Lupus590
Copy link
Contributor

@Lupus590 Lupus590 commented Sep 7, 2017

@Wojbie has a good point, _G will take a while to scroll through, and if I want to see what is in a subtable I could always type table.subtable into the Lua console

@JakobDev
Copy link
Contributor Author

@JakobDev JakobDev commented Sep 7, 2017

@Lupus590 So you think, I should not show the content of subtables?

@JakobDev
Copy link
Contributor Author

@JakobDev JakobDev commented Sep 7, 2017

What did you think, what's the best way, to show, that it is a table who provide a __tostring() and not a string?

btw:
Should I move this function to textutils to give other programs access to it?

@Lupus590
Copy link
Contributor

@Lupus590 Lupus590 commented Sep 7, 2017

Part of the point of the metamethod is that it's discreet about what it's doing.

As for moving to textutils, maybe. I'll have a look at it properly to make up my mind.

Edit: For moving to textutils, I would rename the function to a more generic usecase descriptor. FirstDepthKeySafeSerialise is the name I can come up with at the moment.

@JakobDev
Copy link
Contributor Author

@JakobDev JakobDev commented Sep 11, 2017

Changes are now done!

@@ -19,15 +19,25 @@ local tEnv = {
setmetatable( tEnv, { __index = _ENV } )

local function displayTable( tDisplay )

This comment has been minimized.

@Lupus590

Lupus590 Sep 11, 2017
Contributor

The function is called displayTable but returns a string instead of doing its name sake.

If the only use of the resulting string is to print it (for this program at least) then in my mind the function should do the actual printing. If the output string does get used for something else then this function should be renamed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants