Skip to content

Make getDirectoryItems be 1-indexed #285

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

Merged
merged 1 commit into from
Jul 26, 2025
Merged

Conversation

tobloef
Copy link
Contributor

@tobloef tobloef commented Jul 21, 2025

Forgive me if I'm wrong, I'm new to Lua, but shouldn't the table be 1-indexed? As is right now, a naive iteration through the table or using functions like table.concat will ignore the first item in the directory.

I do realize that this would be a breaking change, so an alternative would also be to add the fact that it's 0-indexed as a warning to the docs.

@tobloef
Copy link
Contributor Author

tobloef commented Jul 24, 2025

Since I have doubts that this will get merged, let me leave this workaround here for anyone finding this issue in the future:

local originalGetDirectoryItems = love.filesystem.getDirectoryItems
function love.filesystem.getDirectoryItems(path)
    local originalItems = originalGetDirectoryItems(path)

    -- Fix for result being 0-indexed
    local newItems = {}
    for i = 0, #originalItems do
        newItems[i + 1] = originalItems[i]
    end

    return newItems
end

@tobloef
Copy link
Contributor Author

tobloef commented Jul 24, 2025

I have also opened a PR to update the docs, which I assume will be easier to get merged 🙂

Edit: I have closed that PR, since it looks like this will get merged 🎉

Copy link
Member

@kivutar kivutar left a comment

Choose a reason for hiding this comment

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

Please check how love2d indexes this before merging.

@tobloef
Copy link
Contributor Author

tobloef commented Jul 26, 2025

Please check how love2d indexes this before merging.

Great idea!

Looking at the LOVE source code, it was not immediately obvious to me. I think they're adding + 1 to the written index on this line:

int w_getDirectoryItems(lua_State *L)
{
	const char *dir = luaL_checkstring(L, 1);
	std::vector<std::string> items;

	instance()->getDirectoryItems(dir, items);

	lua_createtable(L, (int) items.size(), 0);

	for (int i = 0; i < (int) items.size(); i++)
	{
		lua_pushstring(L, items[i].c_str());
		// Adds + 1 to what I assume is the index here
		lua_rawseti(L, -2, i + 1);
	}

	// Return the table.
	return 1;
}

To verify, I wrote a quick test:

function love.draw()
    local files = love.filesystem.getDirectoryItems("")
    table.sort(files)
    
    -- The first item should be "aaa.txt"
    -- For my test setup, the folder just contains "main.lua" and a "aaa.txt"
    local firstFile = files[1]

    if firstFile == "aaa.txt" then
        love.graphics.rectangle("fill", 0, 0, 100, 100)
    else
        love.graphics.circle("fill", 50, 50, 50)
    end
end

Running it with Love and Lutro in Ludo respectively, we can see that the difference.

image

And then the negative test, looking at files[0]:

image

So I would say with confidence that there's a difference between Lutro and at least the latest version of Love.

@kivutar kivutar merged commit 3207ece into libretro:master Jul 26, 2025
12 checks passed
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

Successfully merging this pull request may close these issues.

2 participants