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

[feature request] optionally load null values as nil #31

Open
haizaar opened this issue May 10, 2019 · 7 comments
Open

[feature request] optionally load null values as nil #31

haizaar opened this issue May 10, 2019 · 7 comments

Comments

@haizaar
Copy link

haizaar commented May 10, 2019

Good day,

I'm reading about nil usage in docs and I wonder whether it's reasonable to have a parameter to lyaml.load() function that will make it load null values as nil.

My yamls are usually config files, so they do not have null keys. I would rather prefer for downstream code that uses loaded config to use it simply as table - today it has to assume it's lyaml table for null detection.

What do you think?

@gvvaughan
Copy link
Owner

Hi Haizaar,

You can already do this with the low-level API, much the same as capturing line and column numbers. Adding it to the high-level API is likely to confuse people who don’t understand the difference between YAML null and Lua nil, so not worth it IMHO.

Cheers,
Gary

@haizaar
Copy link
Author

haizaar commented May 11, 2019

Hi Gary,

I see what you mean. Is there example of how it can be done with low-level API? I've looked at the relevant section in README but it looks rather theoretical to casual Lua programmer like myself.

Thank you,
Zaar

@gvvaughan
Copy link
Owner

Sure. In my Specl project on github, I rewrote the libYAML C high-level parser in Lua using the lyaml low-level parser and added in line and column numbers for each token to help with error messages.

You should be able to start with the Specl parser, maybe remove the line number stuff if you don’t need it, and change null parsing to produce nil instead of lyaml.null.

HTH,
Gary

@haizaar
Copy link
Author

haizaar commented May 11, 2019

Hi Gary,

I looked that the implementation, and while it was a good read I found this approach way over-complex for my needs (it will double the LOC count in my openresty scripts :). Hence I simply wrote a function that drops all keys with lyaml.null. Posting it here in hope it will be helpful to others.

P.S. Loved the typecheck lib!

local function null2nil(obj)
    -- cleanup keys with lyaml.null values
    -- in obj table produced by lyaml.load
    -- https://github.com/gvvaughan/lyaml/issues/31
    for k, v in pairs(obj) do
        if v == lyaml.null then
            obj[k] = nil
        end
        if type(v) == "table" then
            null2nil(v)
        end
    end
end

@haizaar haizaar closed this as completed May 11, 2019
@ghost
Copy link

ghost commented Dec 25, 2022

I would like to reopen this. I understand that the proposed low-level solution works, but is kinda not pretty if you just want a workflow using luarocks packages unmodified. The null2nil solution also works, of course, but introduces some unnecessary overhead that I'm not super happy about. Given we already have an options table, would it really be so aesthetically un-pleasing and/or difficult to add a null_as_nil key to that options table?
(That being said, I do appreciate the fact that you as the author might have opinions on what the interface should be like, and what should and should not be supported. But if a bunch of people start wrapping your interface in the same way, it just feels like maybe the interface should just support that directly!)

@gvvaughan
Copy link
Owner

Sure. alet's reopen it for consideration as an option.

Would you like to work on a PR?

@gvvaughan gvvaughan reopened this Jan 7, 2023
@ghost
Copy link

ghost commented Jan 8, 2023

I'll take a look at working on the PR once I'm back from taking a break (burned myself out a little recently), but I actually haven't submitted a PR before (even though I've been writing code for years now, shame on me :P). I think I should be able to manage without problems, but please be patient if I don't! :)

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

No branches or pull requests

2 participants