Can't distinguish between nil variables (that don't exist) and default values #58

Closed
jeremyherbert opened this Issue Oct 28, 2014 · 11 comments

Comments

Projects
None yet
6 participants
@jeremyherbert

It seems unclear to me as to how to deal with variables that do not exist in the lua state. This combined with issue #41 means that lots of things can go bad without the user being informed. I think the correct behaviour here would be an exception, as int cannot be NULL. However it has been noted in #10 that exceptions are not going to be used so I am not sure how to proceed.

$ lua
Lua 5.2.3  Copyright (C) 1994-2013 Lua.org, PUC-Rio
> print(y)
nil
>

#include "Selene/include/selene.h"
#include <string>
#include <iostream>

int main(void) {
    sel::State state;
    state["x"] = 0;
    state["y"] = std::string("");

    int x = state["x"];
    int x2 = state["x2"]; // this doesn't exist

    std::string y = state["y"];
    std::string y2 = state["y2"]; // this doesn't exist

    std::cout << (x == x2 ? "same" : "different") << std::endl;
    std::cout << (y == y2 ? "same" : "different") << std::endl;
    return 0;
}
$ g++-4.9 -std=c++11 -g -I/usr/include/lua5.2 test.cpp -llua5.2 -o test
$ ./test
same
same
$
@jeremyherbert

This comment has been minimized.

Show comment
Hide comment
@jeremyherbert

jeremyherbert Oct 28, 2014

PS I would be happy to attempt to fix this myself if given some guidance as to how to implement it so that it can be merged into the master branch. I assume that fixing this will also fix #41.

PS I would be happy to attempt to fix this myself if given some guidance as to how to implement it so that it can be merged into the master branch. I assume that fixing this will also fix #41.

@jeremyong

This comment has been minimized.

Show comment
Hide comment
@jeremyong

jeremyong Oct 29, 2014

Owner

I'm going to work on adding exception support via C++ exceptions. Initially, I was opposed to this because it would force users of the library to enable exceptions in code. However, I've since realized that exception type handling exists in Lua already in the form of a setjmp/longjmp so there's no sense trying to obscure this fact. I'll keep you posted here. I would happily accept contributions but this isn't a trivial change. That said, don't let that stop you from trying if you're interested in learning!

Owner

jeremyong commented Oct 29, 2014

I'm going to work on adding exception support via C++ exceptions. Initially, I was opposed to this because it would force users of the library to enable exceptions in code. However, I've since realized that exception type handling exists in Lua already in the form of a setjmp/longjmp so there's no sense trying to obscure this fact. I'll keep you posted here. I would happily accept contributions but this isn't a trivial change. That said, don't let that stop you from trying if you're interested in learning!

@jeremyherbert

This comment has been minimized.

Show comment
Hide comment
@jeremyherbert

jeremyherbert Oct 29, 2014

Ok no problems. I will leave it to you, as I think that will be more appropriate. I eagerly await any patches!

Ok no problems. I will leave it to you, as I think that will be more appropriate. I eagerly await any patches!

@hgsteggles

This comment has been minimized.

Show comment
Hide comment
@hgsteggles

hgsteggles Mar 5, 2015

To deal with this I've been doing:

bool exists(sel::Selector selector) {
    std::string str = selector;

    return !(str.compare("") == 0);
}

To deal with this I've been doing:

bool exists(sel::Selector selector) {
    std::string str = selector;

    return !(str.compare("") == 0);
}
@hgsteggles

This comment has been minimized.

Show comment
Hide comment
@hgsteggles

hgsteggles Mar 6, 2015

This doesn't work for booleans though unless you change primitives.h

inline std::string _get(_id<std::string>, lua_State *l, const int index) {
    if (lua_isboolean(l, index) == 1) {
        return std::to_string(lua_toboolean(l, index));
    }
    else {
        size_t size;
        const char *buff = lua_tolstring(l, index, &size);
        return std::string{buff, size};
    }
}

I might have missed something but I think, along with my previous post, this fixes the problem. In my version of Selene I also have a public getter for Selector::_name so that I can print the names of missing variables. The function bool exists(sel::Selector) could be named something more appropriate and you could make it into a public member function of Selector.

This doesn't work for booleans though unless you change primitives.h

inline std::string _get(_id<std::string>, lua_State *l, const int index) {
    if (lua_isboolean(l, index) == 1) {
        return std::to_string(lua_toboolean(l, index));
    }
    else {
        size_t size;
        const char *buff = lua_tolstring(l, index, &size);
        return std::string{buff, size};
    }
}

I might have missed something but I think, along with my previous post, this fixes the problem. In my version of Selene I also have a public getter for Selector::_name so that I can print the names of missing variables. The function bool exists(sel::Selector) could be named something more appropriate and you could make it into a public member function of Selector.

@mamahuhu

This comment has been minimized.

Show comment
Hide comment
@mamahuhu

mamahuhu Feb 25, 2016

I think this feature would be a great addition, whenever one wants to use .lua files as configuration files.

This would allow the detection of a "missing" value in the configuration file, for which the calling code could take user-defined actions (e.g. if a boolean flag was missing in the configuration file, the calling code could choose to initialize it to whether true or false depending on the desired default behaviour of the software).

I think this feature would be a great addition, whenever one wants to use .lua files as configuration files.

This would allow the detection of a "missing" value in the configuration file, for which the calling code could take user-defined actions (e.g. if a boolean flag was missing in the configuration file, the calling code could choose to initialize it to whether true or false depending on the desired default behaviour of the software).

@wileyyugioh

This comment has been minimized.

Show comment
Hide comment
@wileyyugioh

wileyyugioh Oct 24, 2016

If you want to check if a variable exists it is even easier and more universal with lua_isnil.

Just looking at the header for Selector.h, you can add

bool exists()
{
    ResetStackOnScopeExit save(_state);
    _traverse();
    _get();

    return !lua_isnil(_state, -1);
}

wileyyugioh commented Oct 24, 2016

If you want to check if a variable exists it is even easier and more universal with lua_isnil.

Just looking at the header for Selector.h, you can add

bool exists()
{
    ResetStackOnScopeExit save(_state);
    _traverse();
    _get();

    return !lua_isnil(_state, -1);
}
@adam4813

This comment has been minimized.

Show comment
Hide comment
@adam4813

adam4813 Dec 18, 2016

Contributor

@jeremyong Any chance of getting in @wileyyugioh 's solution ? I can submit a PR but they seem to sort of sit there

Contributor

adam4813 commented Dec 18, 2016

@jeremyong Any chance of getting in @wileyyugioh 's solution ? I can submit a PR but they seem to sort of sit there

@jeremyong

This comment has been minimized.

Show comment
Hide comment
@jeremyong

jeremyong Dec 18, 2016

Owner

If you submit it I'll accept it. I know I've been an awful maintainer for this project. Life sort of catches up to you in other ways sorry @adam4813

Owner

jeremyong commented Dec 18, 2016

If you submit it I'll accept it. I know I've been an awful maintainer for this project. Life sort of catches up to you in other ways sorry @adam4813

@adam4813

This comment has been minimized.

Show comment
Hide comment
@adam4813

adam4813 Dec 18, 2016

Contributor

Ok, will get that up later today or tomorrow.

I completely understand how life is more important than pet projects, so don't take my comment as and attempt at being rude/angry please.

Contributor

adam4813 commented Dec 18, 2016

Ok, will get that up later today or tomorrow.

I completely understand how life is more important than pet projects, so don't take my comment as and attempt at being rude/angry please.

@jeremyong

This comment has been minimized.

Show comment
Hide comment
@jeremyong

jeremyong Dec 18, 2016

Owner

Not at all. I'm genuinely being apologetic and wish I had more time haha

Owner

jeremyong commented Dec 18, 2016

Not at all. I'm genuinely being apologetic and wish I had more time haha

adam4813 added a commit to adam4813/Selene that referenced this issue Dec 18, 2016

@jeremyong jeremyong closed this in #168 Jan 1, 2017

ThePhD added a commit to ThePhD/Selene that referenced this issue Aug 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment