-
-
Notifications
You must be signed in to change notification settings - Fork 246
Utility function to handle empty query results and templates #1386
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
Conversation
Hmm. Need to think if the bug isn't really the incompatibility with Lua here. Not sure if we should double down on this being useful. I'd rather make this lua constituent otherwise we're just asking for future trouble. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will be a nice change! I added some minor comments.
website/Space Lua.md
Outdated
@@ -127,10 +129,17 @@ There's a magic `_CTX` global variable available from which you can access usefu | |||
# Lua implementation notes | |||
Space Lua is intended to be a more or less complete implementation of [Lua 5.4](https://www.lua.org/manual/5.4/). However, a few features are (still) missing: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the lead in here should be more inclusive of the following headers. Maybe:
Here are some of the expected differences and planned and not planned features:
website/Space Lua.md
Outdated
* _ENV (planned) | ||
* Full metatable support (only partial now, planned) | ||
## Differences | ||
* empty table `{}` tests false, while in [Lua 5.4](https://www.lua.org/manual/5.4/manual.html#3.3.4) "All values different from **nil** nad **false** test true". However this has a really nice interaction with empty [[Space Lua/Lua Integrated Query|query]] results |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo on nad
in the docs quote.
website/Space Lua.md
Outdated
## Planned | ||
* _ENV | ||
* Full [metatable](https://www.lua.org/manual/5.4/manual.html#2.4) support (only partial now) | ||
* Complete [[API/string]] API (some patterns in `gmatch` don’t work correctly) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a section to [[API/string]]
about how Lua patterns and Space Lua patterns have some key differences, not just for gmatch
. What do you think about removing:
(some patterns in
gmatch
don’t work correctly)
and adding a bullet to the Differences section above, maybe:
- lua patterns are translated to Javascript regex, so there are some [[API/string|key differences]].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably same comment applies as the empty table truthiness. See link in #1284, maybe we can actually get 100% compatibility?
Essentially we would like to have some behavior like ?? operator. A Lua-compatible solution would be to add a function that checks for truthiness in a different way, appropriate for content:
I'll keep in mind that we're aiming for full Lua compatibility. Then a fully Lua-compatible way would be Yet another conformant solution could be to use metatables to override an operation on the query result. By setting our custom method for
I don't have a strong opinion on which operator to use, I went with XOR because "only one of the inputs is true" This would be completely normal Lua, and even shorter syntax than now (by 1 character xD). I'm not worried about making query function inconsistent with expectations, since it already has special parsing rules, but it may confuse new users as to what Bonus: Maybe this would be a path for composing queries together using some other overridable operators? Like |
I like the |
Moved documentation for 'each' function to non-standard section.
The scope has changed a bit, but I thought that it's better to just reorganize this PR to keep the discussion about possible options. I think this is a more elegant solution, are we OK to remove #1359? Everyone is supposed to know this is unstable version, and I don't recall any changelog or forum talk about that feature. |
Very nice. I like this solution. And indeed I hope/assume people will not have used the extra template.each argument yet, and if so they'll figure it out 😄 |
More general implementation of #1359
This also documents current workaround for #1343