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

update utils; enum + kpairs #413

Merged
merged 3 commits into from
Feb 13, 2022
Merged

update utils; enum + kpairs #413

merged 3 commits into from
Feb 13, 2022

Conversation

Tieske
Copy link
Member

@Tieske Tieske commented Jan 13, 2022

adds:

  • feature: utils.enum accept hash tables as well
  • feature: utils.kpairs iterator over all non-integer keys (inverse of ipairs)

@Tieske Tieske force-pushed the enum branch 2 times, most recently from 2603b81 to b5ad58a Compare January 13, 2022 19:37
@kalekje
Copy link
Contributor

kalekje commented Jan 17, 2022

...should we add kpairs to tablex then?

@tst2005

This comment was marked as resolved.

@alerque alerque changed the title updaet utils; enum + kpairs update utils; enum + kpairs Jan 17, 2022
@alerque
Copy link
Member

alerque commented Jan 17, 2022

...should we add kpairs to tablex then?

Putting it in utils next to the existing utils.npairs does seem like the most consistent thing. That being said I would probably me most likely to look for it in tablex.

@Tieske
Copy link
Member Author

Tieske commented Jan 27, 2022

we could move all of them; pack, unpack, npairs and kpairs to the tablex module. The main problem we'd have with that is probably the dependency tree (inter dependencies between PL modules).

I think @stevedonovan made an effort to decouple them, such that most would rely only on utils and compat. If we move them then we'd undo that since pack and unpack are used all over the place iirc.

@kalekje
Copy link
Contributor

kalekje commented Jan 28, 2022

Thinking about it some more, it does make sense it's in utils. I'd probably just use kpairs = untils.kpairs and with unpack anyway.

@Tieske
Copy link
Member Author

Tieske commented Jan 29, 2022

we can also add them to utils, but include an alias in tablex. Such that they are where users expect them, without adding extra dependencies.

@alerque
Copy link
Member

alerque commented Jan 29, 2022

I don't think we need an alias, but a nod in the tablex documentation to the additional table related utilities in utils in case one comes searching for them might be useful.

@Tieske
Copy link
Member Author

Tieske commented Jan 30, 2022

@alerque any specific reason you don't like an alias? There's already plenty of them, see the file module for example.

(an Alias might also make the docs easier, since we can attach Ldoc comments to an alias)

@alerque
Copy link
Member

alerque commented Feb 11, 2022

If there is already precedent to get docs then okay, just go for it.

My personal dislike of multiple aliases for the same code path is rooted in history with other projects where the difficulty of refactoring things later goes up the more ways there are of accessing the same code. For example in this case when looking for uses in a code base you can't just grep the Lua code for utils.kpairs, you'll have to grep for (utils|tablex).kpairs. In this case "kpairs" is likely to be unique anyway, but the same is not true for most API calls.

@kalekje
Copy link
Contributor

kalekje commented Feb 11, 2022

To be fair, we don't call pairs or ipairs like table.pairs() or x:pairs() anyways, so it's not really clear to me why kpairs should be an exception.

@Tieske
Copy link
Member Author

Tieske commented Feb 13, 2022

merging this for now, at some point (before 2.0) we probably need to revisit some of the function in utils anyway, and the module separation (interdependencies)

@Tieske Tieske merged commit 57253b2 into master Feb 13, 2022
@Tieske Tieske deleted the enum branch February 13, 2022 08:42
bungle added a commit to Kong/kong that referenced this pull request Aug 8, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
bungle added a commit to Kong/kong that referenced this pull request Aug 8, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
StarlightIbuki pushed a commit to Kong/kong that referenced this pull request Aug 9, 2022
### Summary

#### 1.13.1 (2022-Jul-22)
 - fix: `warn` unquoted argument
   [#439](lunarmodules/Penlight#439)

#### 1.13.0 (2022-Jul-22)
 - fix: `xml.parse` returned nonsense when given a file name
   [#431](lunarmodules/Penlight#431)
 - feat: `app.require_here` now follows symlink'd main modules to their directory
   [#423](lunarmodules/Penlight#423)
 - fix: `pretty.write` invalid order function for sorting
   [#430](lunarmodules/Penlight#430)
 - fix: `compat.warn` raised write guard warning in OpenResty
   [#414](lunarmodules/Penlight#414)
 - feat: `utils.enum` now accepts hash tables, to enable better error handling
   [#413](lunarmodules/Penlight#413)
 - feat: `utils.kpairs` new iterator over all non-integer keys
   [#413](lunarmodules/Penlight#413)
 - fix: `warn` use rawget to not trigger strict-checkers
   [#437](lunarmodules/Penlight#437)
 - fix: `lapp` provides the file name when using the default argument
   [#427](lunarmodules/Penlight#427)
 - fix: `lapp` positional arguments now allow digits after the first character
   [#428](lunarmodules/Penlight#428)
 - fix: `path.isdir` windows root directories (including drive letter) were not considered valid
   [#436](lunarmodules/Penlight#436)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants