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

table.Print - a better version of PrintTable #2033

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Be1zebub
Copy link
Contributor

@Be1zebub Be1zebub commented Nov 30, 2023

A better version of PrintTable, with lua syntax based output.

I would replace PrintTable, but im not sure Rubat would approve of that.
This does not break the behavior and cant cause any backward compatibility issues - because its just a print, so this is a good way to go.

there is also serializer local str = table.ToPlain( tbl, nopretty )
its useful for things like configs

How it looks:

with nopretty:


compare with PrintTable

table.Print is much cleaner, it visually separates several tables and also displays data types correctly - this makes it easy to distinguish "true" from true, Angle(1) from Vector(1), "hello" from "hello " .

upd:

Here's the new version, I've added community suggestions to it:
image

A better version of PrintTable
@Aws0mee
Copy link
Contributor

Aws0mee commented Dec 1, 2023

I think this is a really nice improvement! I've been confused before by the current PrintTable in some scenarios, specifically when dealing with tables in tables. I think you could improve this even further by adding different colors for keys/values and maybe different colors for different types if you want.

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 1, 2023

it would be fun to make the output with syntax highlighting, but i don't think its a good idea.

@Aws0mee
Copy link
Contributor

Aws0mee commented Dec 1, 2023

it would be fun to make the output with syntax highlighting, but i don't think its a good idea.

Why don't you think it's a good idea? I've seen a good amount of people make their own table printing functions similar to yours, but most of them with color. I think it just makes it a little easier to read.

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 1, 2023

I think its cool, but not necessary feature.
it would also inflate the code, turning it into a separate module with 300+ lines - I think we need cleaner output, not a separate overloaded module.

making syntax highlighting is not so difficult, I know some techniques todo it effectively.
I can implement it if the community will support this feature.

@Aws0mee
Copy link
Contributor

Aws0mee commented Dec 1, 2023

I think its cool, but not necessary feature. it would also inflate the code, turning it into a separate module with 300+ lines - I think we need cleaner output, not a separate overloaded module.

making syntax highlighting is not so difficult, I know some techniques todo it effectively. I can implement it if the community will support this feature.

I think the community will support it. You can look at Luvit's p() function for example. I think the colors on it are quite useful for quickly determining the type just by glancing at different values in the table.

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 3, 2023

I really like this, 1000 times better than PrintTable. Also, I suggest localizing the result of formaters[ type( val ) ] in the tostringGmodTypes function:

local format = formaters[ type( val ) ]

if ( format ) then
	return format(val)
end

So you don't need to call type twice.

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 3, 2023

Minor cleanups, add facepunch styling [key] > [ key ], add "if syntax-valid keyname" statement.

How it looks:

@DBotThePony
Copy link
Contributor

DBotThePony commented Dec 6, 2023

I think this is a really nice improvement! I've been confused before by the current PrintTable in some scenarios, specifically when dealing with tables in tables. I think you could improve this even further by adding different colors for keys/values and maybe different colors for different types if you want.

Screenshot_20231206-101122_Firefox Beta

:trollface:

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 6, 2023

Syntax highlighting has been added!
image

table.ToPlain was deleted because it conflicts with new mechanics of work.
nopretty arg is removed, it made sense for serialization but not for print.
added Player & Entity classes support.
I also moved the code to a separate module.

Source table:
Serverside:

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 6, 2023

I think the syntax highlighting should be an optional argument; it could get confusing on what realm the values are defined in

@Aws0mee
Copy link
Contributor

Aws0mee commented Dec 6, 2023

I think it looks really nice. This will finally remove the need for external libraries for this. Hopefully this can get merged.

I think the syntax highlighting should be an optional argument; it could get confusing on what realm the values are defined in

I don't see how this can get confusing. If you call the function serverside then it will print serverside values, if you call it clientside then it will print clientside values. However, I think PrintTable should be kept as well to avoid breaking old code or for people who just want more basic functionality.

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 6, 2023

I think it looks really nice. This will finally remove the need for external libraries for this. Hopefully this can get merged.

I think the syntax highlighting should be an optional argument; it could get confusing on what realm the values are defined in

I don't see how this can get confusing. If you call the function serverside then it will print serverside values, if you call it clientside then it will print clientside values. However, I think PrintTable should be kept as well to avoid breaking old code or for people who just want more basic functionality.

Fair enough, I looked at it from a "calling it shared" view. Just so used to the blue/yellow colors in console lol

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 6, 2023

But yeah, this looks nice. 👍

@robotboy655 robotboy655 added the Addition The pull request adds new functionality. label Dec 8, 2023
@Be1zebub
Copy link
Contributor Author

I was currently developing a grid inventory system, and I realized that I made a few mistakes while making rework that adds colors
image

the current version has few issues, rn im working on a fix and more thorough testing.

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 17, 2023

solved
image
hl2_ihuqyxzbwT
I tested this on 10 large tables from _G from large addons like sam admin mod, it works well rn.

I would also like to ask the community to test this with different tables/on different operating systems - I only did tests in windows client, linux srcds, linux srcds + pterodactyl.

@Be1zebub
Copy link
Contributor Author

Now the multiline strings output has a syntax correct.
image
image

@CornerPin
Copy link
Contributor

You can use string.format's q specifier to make syntactically correct strings. Currently, this doesn't handle cases when a string contains ]], ", \, or any non-printable characters.

@Be1zebub
Copy link
Contributor Author

You can use string.format's q specifier to make syntactically correct strings. Currently, this doesn't handle cases when a string contains ]], ", \, or any non-printable characters.

nah, it does not handle newlines properly.
I think I should add [[, ", \12345 escaping manually.

image
image

@CornerPin
Copy link
Contributor

CornerPin commented Dec 17, 2023

"Hello\
world"

is valid Lua syntax, though I think \n would look prettier. You could fix this with string.gsub.

string.gsub(str, "\\\n", "\\n")

@Be1zebub
Copy link
Contributor Author

Be1zebub commented Dec 17, 2023

"Hello\
world"

is valid Lua syntax, though I think \n would look prettier. You could fix this with string.gsub.

no, it doesnt.

string.gsub(str, "\\\n", "\\n")

nope, it doesnt works.

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 25, 2023

Small suggestion: Localize type at the top to avoid errors from bad addons overriding the function

@Be1zebub
Copy link
Contributor Author

why should anybody care about shitty addons? just uninstall it, if its broken.

@Zaurzo
Copy link
Contributor

Zaurzo commented Dec 25, 2023

why should anybody care about shitty addons? just uninstall it, if its broken.

Fair enough.

@Kefta
Copy link
Contributor

Kefta commented Dec 26, 2023

It's a library file so all global funcs should be localised but that's for another PR.

@garryspins
Copy link

garryspins commented Jan 6, 2024

love this, wrote this myself many years ago and has been a massive help, even added the function src file on my own version, good pr.

@Aspect12
Copy link

Aspect12 commented Apr 6, 2024

This is great, I hope it gets merged soon, either as a direct replacement to PrintTable() or as a separate function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Addition The pull request adds new functionality.
Projects
None yet
9 participants