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

Pretty Encoding #32

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

Pretty Encoding #32

wants to merge 7 commits into from

Conversation

Ayplow
Copy link

@Ayplow Ayplow commented Aug 5, 2019

Related to #11

This is my shot at an efficient pretty encoding facility. I'm happy to implement any changes that could improve performance.

  • A cache is built from the cache argument, avoiding recreation of indent strings. This will consume memory (that will be released with the encoder), however the footprint is minimal
  • Any encoders with the same indent share a cache. Only one assignment is made on depth change.
  • space is equivalent to the third argument of javascript's JSON.stringify

Any encoders with the same indent share a cache. Only one assignment is made on depth change.

`space` is equivalent to the third argument of [javascript's JSON.stringify](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify)
@Ayplow Ayplow changed the title First implementation of pretty encode Pretty Encoding Aug 5, 2019
@tst2005
Copy link
Contributor

tst2005 commented Aug 6, 2019

You are using _ENV... is your changes breaks the Lua 5.1 compat ?

@Ayplow
Copy link
Author

Ayplow commented Aug 6, 2019

Hah right, forgot to write the compat. I'll get on that

@tst2005
Copy link
Contributor

tst2005 commented Aug 7, 2019

Can't you just use a local table items instead of separated variables in _ENV ?

for sample

local config
...
-builder[i] = start_array
+builder[i] = config.start_array
...
- 	if type(space) ~= "string" or space == "" then _ENV = basic else
+ 	if type(space) ~= "string" or space == "" then config = basic else

and just avoid to use _ENV and ugly compat ?

Copy link
Contributor

@tst2005 tst2005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: rename tmp to arraylen ?

Turns out using the environment has zero performance benefit in any version of lua, so it just serves to make the code less readable
This brings the interface more in line with the rest of the library
@tst2005
Copy link
Contributor

tst2005 commented Aug 7, 2019

  • do you use somewhere the f_string_esc_pat introduce with the lua 5.1 compat ?
  • do you lost the original local _ENV = nil ? (I don't know if it is really important)

This doesn't really do anything for consumers, however it is valuable for ensuring variables are made local
@Ayplow
Copy link
Author

Ayplow commented Aug 7, 2019

The string escape pattern is used in f_string? I don't understand what you mean.

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