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

corrade-rc: allow shorthand filename specification #163

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
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
21 changes: 20 additions & 1 deletion src/Corrade/Utility/Implementation/ResourceCompile.h
Original file line number Diff line number Diff line change
Expand Up @@ -304,9 +304,28 @@ Containers::String resourceCompileFrom(const Containers::StringView name, const
}

/* Load all files */
std::vector<Containers::StringView> filenames = conf.values<Containers::StringView>("filename");
std::vector<const ConfigurationGroup*> files = conf.groups("file");
Containers::Array<FileData> fileData;
arrayReserve(fileData, files.size());
arrayReserve(fileData, filenames.size() + files.size());

/* Process loose filename options -- they have no aliases and always
inherit the global options */
for(const Containers::StringView filename: filenames) {
if(filename.isEmpty()) {
Error() << " Error: filename" << fileData.size() + 1 << "in group" << group << "is empty";
return {};
}

Containers::Optional<Containers::Array<char>> contents = Path::read(Path::join(path, filename));
if(!contents) {
Error() << " Error: cannot open file" << filename << "of file" << fileData.size()+1 << "in group" << group;
return {};
}
arrayAppend(fileData, InPlaceInit, filename, globalNullTerminated, globalAlign, *std::move(contents));
}

/* Process [file] groups */
for(const ConfigurationGroup* const file: files) {
const Containers::StringView filename = file->value<Containers::StringView>("filename");
const Containers::StringView alias = file->hasValue("alias") ? file->value<Containers::StringView>("alias") : filename;
Expand Down
22 changes: 22 additions & 0 deletions src/Corrade/Utility/Resource.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ Resource::Resource(const Containers::StringView group): _group{findGroup(group)}
<< "Utility::Resource: group '" << Debug::nospace << group << Debug::nospace << "' overridden with '" << Debug::nospace << overridden->second << Debug::nospace << "\'";
_overrideGroup = new OverrideData(overridden->second);

// TODO this gets printed also if the overriden file didn't even exist, FFS
if(_overrideGroup->conf.value<Containers::StringView>("group") != group) Warning{}
<< "Utility::Resource: overridden with different group, found '"
<< Debug::nospace << _overrideGroup->conf.value<Containers::StringView>("group")
Expand Down Expand Up @@ -216,6 +217,27 @@ Containers::StringView Resource::getString(const Containers::StringView filename

/* Load the file and save it for later use. Linear search is not an
issue, as this shouldn't be used in production code anyway. */

/* Loose filenames first */
const std::vector<Containers::StringView> filenames = _overrideGroup->conf.values<Containers::StringView>("filename");
for(const Containers::StringView name: filenames) {
if(name != filename) continue;

/* Load the file */
Containers::Optional<Containers::Array<char>> data = Path::read(Path::join(Path::split(_overrideGroup->conf.filename()).first(), filename));
if(!data) {
Error{} << "Utility::Resource::get(): cannot open file" << name << "from overridden group";
break;
}

/* Save the file for later use and return. Use a filename from the
compiled-in resources which is guaranteed to be global to avoid
allocating a new string */
it = _overrideGroup->data.emplace(Implementation::resourceFilenameAt(_group->positions, _group->filenames, i), *std::move(data)).first;
return Containers::ArrayView<const char>{it->second};
}

/* Then [file] groups */
const std::vector<const ConfigurationGroup*> files = _overrideGroup->conf.groups("file");
for(const ConfigurationGroup* const file: files) {
const Containers::StringView name = file->hasValue("alias") ? file->value<Containers::StringView>("alias") : file->value<Containers::StringView>("filename");
Expand Down
2 changes: 2 additions & 0 deletions src/Corrade/Utility/Test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -395,9 +395,11 @@ corrade_add_test(UtilityResourceCompileTest
ResourceTestFiles/resources-alignment-larger-than-data-size.conf
ResourceTestFiles/resources-empty-alias.conf
ResourceTestFiles/resources-empty-filename.conf
ResourceTestFiles/resources-empty-file-filename.conf
ResourceTestFiles/resources-empty-group.conf
ResourceTestFiles/resources-no-group.conf
ResourceTestFiles/resources-nonexistent.conf
ResourceTestFiles/resources-file-nonexistent.conf
ResourceTestFiles/resources-nothing.conf
ResourceTestFiles/resources-npot-align.conf
ResourceTestFiles/resources-npot-global-align.conf
Expand Down
7 changes: 6 additions & 1 deletion src/Corrade/Utility/Test/ResourceCompileTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,18 @@ const struct {
} CompileFromInvalidData[]{
{"nonexistent resource file", "/nonexistent.conf",
"file /nonexistent.conf does not exist"},
{"nonexistent file", "resources-nonexistent.conf",
{"nonexistent filename", "resources-nonexistent.conf",
/* There's an error message from Path::read() before */
"\n Error: cannot open file /nonexistent.dat of file 1 in group name\n"},
{"nonexistent [file] filename", "resources-file-nonexistent.conf",
/* There's an error message from Path::read() before */
"\n Error: cannot open file /nonexistent.dat of file 1 in group name\n"},
/* Empty group= option is allowed, tested in compileFromEmptyGroup() */
{"empty group", "resources-no-group.conf",
"group name is not specified"},
{"empty filename", "resources-empty-filename.conf",
"filename 2 in group name is empty"},
{"empty [file] filename", "resources-empty-file-filename.conf",
"filename or alias of file 1 in group name is empty"},
{"empty alias", "resources-empty-alias.conf",
"filename or alias of file 1 in group name is empty"},
Expand Down
2 changes: 2 additions & 0 deletions src/Corrade/Utility/Test/ResourceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ void ResourceTest::overrideGroup() {
/* Two subsequent calls should point to the same location (the file doesn't
get read again) */
CORRADE_VERIFY(rs.getString("predisposition.bin").data() == predisposition.data());

// TODO test both filename= and [file]
}

void ResourceTest::overrideGroupNonexistent() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
group=name

[file]
filename=
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
group=name

[file]
filename=empty.bin
filename=
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
group=name

[file]
filename=/nonexistent.dat
alias=but-it-exists.dat
Original file line number Diff line number Diff line change
@@ -1,5 +1,2 @@
group=name

[file]
filename=/nonexistent.dat
alias=but-it-exists.dat
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
group=test

filename=consequence2.txt

[file]
filename=../ResourceTestFiles/predisposition2.txt
alias=predisposition.bin

[file]
filename=consequence2.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
group=spaces

[file]
# In this case the spaces should be trimmed from the beginning and the end, but
# not from inside the quotes, DO NOT CLEAN UP THE TRAILING SPACE
# vvv----------------------vvv
filename= "name with spaces.txt"

[file]
# Here's a leading and trailing space, DO NOT CLEAN UP
# v------------------v
filename= predisposition.bin
Expand Down
3 changes: 3 additions & 0 deletions src/Corrade/Utility/Test/ResourceTestFiles/resources.conf
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@ group=test
filename=../ResourceTestFiles/predisposition.bin
alias=predisposition.bin

# Loose filename= options are tested in resources-spaces.conf and
# resources-overriden.conf but not here because this tests that the filenames
# get correctly sorted
[file]
filename=consequence.bin