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

Expose Alias API to Lua #3423

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions library/LuaApi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3577,6 +3577,67 @@ static int internal_md5file(lua_State *L)
}
}

/**************************
* Wrappers for Alias API *
**************************/

static int internal_getAliasCommand(lua_State *L)
{
const char *name = luaL_checkstring(L, 1);
std::string ret = Core::getInstance().GetAliasCommand(name);
if (!ret.empty())
lua_pushstring(L, ret.c_str());
else
lua_pushnil(L);
return 1;
}

static int internal_listAliases(lua_State *L)
{
auto aliases = Core::getInstance().ListAliases();
Copy link
Member

Choose a reason for hiding this comment

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

If we add a Lua::Push override for std::vector, this function could be reduced to Lua::Push(L, Core::getInstance().ListAliases()); return 1;

Copy link
Author

Choose a reason for hiding this comment

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

Nice - will look at this.


lua_newtable(L);
for (const auto& p : aliases)
{
Lua::Push(L, p.first.c_str()); // alias
Lua::PushVector(L, p.second);
lua_settable(L, -3);
}
return 1;
}

static int internal_isAlias(lua_State *L)
{
const char *name = luaL_checkstring(L, 1);
lua_pushboolean(L, Core::getInstance().IsAlias(name));
return 1;
}

static int internal_addAlias(lua_State *L)
{
std::vector<std::string> alias;
Lua::GetVector(L, alias, 1);

if (!alias.empty())
{
std::string name = alias[0];
Copy link
Member

Choose a reason for hiding this comment

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

I think we should match the AddAlias() signature instead by taking two arguments: name (a string) and arguments (a list of arguments). It looks like you're retrieving the alias name from the first element of the list here, which doesn't match the C++ API.

Copy link
Author

Choose a reason for hiding this comment

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

I'll adjust.

alias.erase(alias.begin());
lua_pushboolean(L, Core::getInstance().AddAlias(name, alias));
} else {
lua_pushboolean(L, false);
}

return 1;
}

static int internal_removeAlias(lua_State *L)
{
const char *rem_alias = luaL_checkstring(L, 1);

lua_pushboolean(L, Core::getInstance().RemoveAlias(rem_alias));
return 1;
}

static const luaL_Reg dfhack_internal_funcs[] = {
{ "getPE", internal_getPE },
{ "getMD5", internal_getmd5 },
Expand Down Expand Up @@ -3605,6 +3666,13 @@ static const luaL_Reg dfhack_internal_funcs[] = {
{ "getCommandDescription", internal_getCommandDescription },
{ "threadid", internal_threadid },
{ "md5File", internal_md5file },

{ "getAliasCommand", internal_getAliasCommand },
{ "isAlias", internal_isAlias },
{ "listAliases", internal_listAliases },
{ "addAlias", internal_addAlias },
{ "removeAlias", internal_removeAlias },

{ NULL, NULL }
};

Expand Down
11 changes: 11 additions & 0 deletions test/library/alias.lua
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-- Simple tests to ensure the alias functions work as expected
function test.aliases()
expect.eq(false, dfhack.internal.isAlias("foo"))
Copy link
Member

Choose a reason for hiding this comment

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

this can actually be flaky since it depends on the current active environment. these function tests allow you to specify a setup/teardown wrapper where you can save the current aliases, clear the alias list for the test, and restore the original aliases afterwards. See the orders plugin tests for an example.

expect.eq(true, dfhack.internal.addAlias({"foo", "help"}))
expect.eq(true, dfhack.internal.isAlias("foo"))
expect.eq("help", dfhack.internal.getAliasCommand("foo"))
expect.eq("this is not an alias", dfhack.internal.getAliasCommand("this is not an alias"))
expect.eq("help", dfhack.internal.listAliases()["foo"][1])
expect.eq(true, dfhack.internal.removeAlias("foo"))
expect.eq(false, dfhack.internal.isAlias("foo"))
end