diff --git a/src/TLuaInterpreter.cpp b/src/TLuaInterpreter.cpp index 7d2d9bc41..81be5f594 100644 --- a/src/TLuaInterpreter.cpp +++ b/src/TLuaInterpreter.cpp @@ -7260,6 +7260,22 @@ int TLuaInterpreter::getProfileInformation(lua_State* L) return 1; } +// No documentation available in wiki - internal function +// The folder a profile name resolves to, or an empty string if there is no such +// profile. For writers, and so stricter than mudlet::getCanonicalProfileName(), +// which also resolves a game Mudlet ships with that has never been opened: +// writeProfileData() creates whatever folder it is handed, so writing under such +// a name would turn that game into a profile of its own. Readers want the looser +// call. +static QString canonicalProfileFolder(const QString& profileName) +{ + const QString folder = mudlet::self()->getCanonicalProfileName(profileName); + if (folder.isEmpty() || !QDir(mudlet::getMudletPath(enums::profileHomePath, folder)).exists()) { + return QString(); + } + return folder; +} + // Documentation: https://wiki.mudlet.org/w/Manual:Miscellaneous_Functions#setProfileInformation int TLuaInterpreter::setProfileInformation(lua_State* L) { @@ -7278,18 +7294,20 @@ int TLuaInterpreter::setProfileInformation(lua_State* L) if (params == 1) { text = lua_tostring(L, 1); } else { - profileName = lua_tostring(L, 1); + const QString requestedName = lua_tostring(L, 1); + profileName = canonicalProfileFolder(requestedName); + if (profileName.isEmpty()) { + return warnArgumentValue(L, __func__, qsl("profile '%1' does not exist").arg(requestedName)); + } text = lua_tostring(L, 2); } - QPair result = mudlet::self()->writeProfileData(profileName, qsl("description"), text); - int returnCode = 1; - lua_pushboolean(L, result.first); - if (!result.second.isEmpty()) { - lua_pushfstring(L, "setProfileInformation: %s does not exist", profileName.toUtf8().constData()); - returnCode = 2; + const QPair result = mudlet::self()->writeProfileData(profileName, qsl("description"), text); + if (!result.first) { + return warnArgumentValue(L, __func__, result.second); } - return returnCode; + lua_pushboolean(L, true); + return 1; } // Documentation: https://wiki.mudlet.org/w/Manual:Miscellaneous_Functions#clearProfileInformation @@ -7300,7 +7318,14 @@ int TLuaInterpreter::clearProfileInformation(lua_State* L) return lua_error(L); } - QString profileName = (params > 0) ? QString{lua_tostring(L, 1)} : getHostFromLua(L).getName(); + QString profileName = getHostFromLua(L).getName(); + if (params > 0) { + const QString requestedName = lua_tostring(L, 1); + profileName = canonicalProfileFolder(requestedName); + if (profileName.isEmpty()) { + return warnArgumentValue(L, __func__, qsl("profile '%1' does not exist").arg(requestedName)); + } + } QString desc = ""; // if this is a default game, return to the orginal text @@ -7311,14 +7336,12 @@ int TLuaInterpreter::clearProfileInformation(lua_State* L) } } - QPair result = mudlet::self()->writeProfileData(profileName, qsl("description"), desc); - int returnCode = 1; - lua_pushboolean(L, result.first); - if (!result.second.isEmpty()) { - lua_pushstring(L, "Profile not found"); - returnCode = 2; + const QPair result = mudlet::self()->writeProfileData(profileName, qsl("description"), desc); + if (!result.first) { + return warnArgumentValue(L, __func__, result.second); } - return returnCode; + lua_pushboolean(L, true); + return 1; } // Internal function - helper for updateColorTable(). @@ -8648,13 +8671,12 @@ int TLuaInterpreter::setSaveCommandHistory(lua_State* L) // profile: return warnArgumentValue(L, __func__, "disabled by profile global preference"); } + // both defaults have to stand outside the argument handling below: + // setSaveCommandHistory() and setSaveCommandHistory(name) each turn saving + // on, so neither belongs inside a branch on the argument count: const char* name = "main"; bool saveCommands = true; - // if there is no arguments we will set the "save command history" on the - // main command line: - if (n == 1) { - saveCommands = getVerifiedBool(L, __func__, 1, "save command history", true); - } else { + if (n > 0) { if (lua_type(L, 1) == LUA_TSTRING) { // First argument is a string so is presumably a command line name name = CMDLINE_NAME(L, 1); diff --git a/src/mudlet-lua/lua/Other.lua b/src/mudlet-lua/lua/Other.lua index 4f2a2b688..707e7c60a 100644 --- a/src/mudlet-lua/lua/Other.lua +++ b/src/mudlet-lua/lua/Other.lua @@ -1113,7 +1113,11 @@ local acceptableSuffix = {"xml", "mpackage", "zip", "trigger"} function verbosePackageInstall(fileName) local ok, err = installPackage(fileName) - local packageName = string.gsub(fileName, getMudletHomeDir() .. "/", "") + -- this has to stay a literal prefix strip: as a Lua pattern the profile path's + -- magic characters bite, and a "-" (as in "Mudlet self-test") stops it + -- matching at all + local profileFolder = getMudletHomeDir() .. "/" + local packageName = fileName:starts(profileFolder) and fileName:sub(#profileFolder + 1) or fileName -- That is all for installing, now to announce the result to the user: mudlet.Locale = mudlet.Locale or loadTranslations("Mudlet") if ok then diff --git a/src/mudlet-lua/tests/Miscallaneous_spec.lua b/src/mudlet-lua/tests/Miscallaneous_spec.lua index cf45d219b..e5394daa4 100644 --- a/src/mudlet-lua/tests/Miscallaneous_spec.lua +++ b/src/mudlet-lua/tests/Miscallaneous_spec.lua @@ -337,6 +337,21 @@ describe("Tests C++ functions in the Miscallaneous category", function() -- what it found: the self-test profile is reused between runs. local descriptionFile = getMudletHomeDir() .. "/description" + -- A game Mudlet lists in the connection dialog that has no folder here, or + -- nil if they all have one. Such a name resolves for a profile lookup + -- without being a profile, which is the case worth testing. getProfiles() + -- lists folders, so a bundled game missing from it has none; several are + -- offered so that a run against a config where some have been opened still + -- finds one. + local function unopenedBundledGame() + local profiles = getProfiles() + for _, game in ipairs({"Achaea", "Aetolia", "Lusternia", "Imperian", "StickMUD", "Materia Magica"}) do + if not profiles[game] then + return game + end + end + end + local function restoreDescription() local original = getProfileInformation() -- a profile that has never had a description has no file for one, and @@ -397,6 +412,16 @@ describe("Tests C++ functions in the Miscallaneous category", function() assert.equals("named form", getProfileInformation()) end) + it("matches the profile whatever case it is named in", function() + finally(restoreDescription()) + + assert.is_true(setProfileInformation(getProfileName():upper(), "shouted form")) + assert.equals("shouted form", getProfileInformation()) + -- naming the profile in the wrong case must find the folder it has, + -- not make a second one beside it + assert.is_nil(getProfiles()[getProfileName():upper()]) + end) + it("is what getProfiles reports as the description", function() finally(restoreDescription()) setProfileInformation("as seen by getProfiles") @@ -405,14 +430,32 @@ describe("Tests C++ functions in the Miscallaneous category", function() end) it("refuses a profile that does not exist", function() - -- BUG: writeProfileData() creates the profile folder it is given, so - -- naming a profile that is not there makes one, description file and - -- all - a phantom that the connection dialog and getProfiles() then - -- both list. Left pending rather than pinning it as correct. - pending("setProfileInformation() creates a folder for a profile that does not exist") local ok, err = setProfileInformation("mudlet-spec-never-a-profile", "text") - assert.is_false(ok) - assert.is_string(err) + assert.is_nil(ok) + assert.equals("profile 'mudlet-spec-never-a-profile' does not exist", err) + -- refusing is not enough on its own: the write goes through + -- writeProfileData(), which creates whatever folder it is handed, and + -- a folder here is a profile to getProfiles() and the connection dialog + local profiles = getProfiles() + assert.is_table(profiles[getProfileName()], "getProfiles() answered nothing at all") + assert.is_nil(profiles["mudlet-spec-never-a-profile"]) + end) + + it("refuses a game Mudlet ships with that has no profile of its own", function() + local game = unopenedBundledGame() + if not game then + pending("every bundled game this spec knows of has a profile here") + return + end + + -- the getter answers for this name, which is what makes it the + -- bundled-game case rather than a second unknown-name spec + assert.is_string(getProfileInformation(game)) + + local ok, err = setProfileInformation(game, "text") + assert.is_nil(ok) + assert.equals(("profile '%s' does not exist"):format(game), err) + assert.is_nil(getProfiles()[game]) end) end) @@ -422,19 +465,37 @@ describe("Tests C++ functions in the Miscallaneous category", function() end) it("refuses a profile that does not exist", function() - -- BUG: the same as setProfileInformation's - the write creates the - -- folder it was told to write into, so clearing the description of a - -- profile that is not there conjures one up. - pending("clearProfileInformation() creates a folder for a profile that does not exist") local ok, err = clearProfileInformation("mudlet-spec-never-a-profile") - assert.is_false(ok) - assert.is_string(err) + assert.is_nil(ok) + assert.equals("profile 'mudlet-spec-never-a-profile' does not exist", err) + assert.is_nil(getProfiles()["mudlet-spec-never-a-profile"]) + end) + + it("refuses a game Mudlet ships with that has no profile of its own", function() + local game = unopenedBundledGame() + if not game then + pending("every bundled game this spec knows of has a profile here") + return + end + + assert.is_string(getProfileInformation(game)) + + local ok, err = clearProfileInformation(game) + assert.is_nil(ok) + assert.equals(("profile '%s' does not exist"):format(game), err) + -- clearing writes the description the game ships with, so a folder + -- made here would not merely exist, it would read as a set up profile + assert.is_nil(getProfiles()[game]) end) it("puts back the description a bundled game ships with", function() finally(restoreDescription()) setProfileInformation("something else entirely") + -- both forms have to restore the blurb, so the named one clears first + -- and the description is dirtied again for the no-argument one + assert.is_true(clearProfileInformation(getProfileName())) + setProfileInformation("something else entirely") assert.is_true(clearProfileInformation()) -- the self-test profile is one of Mudlet's own games, so clearing @@ -474,24 +535,49 @@ describe("Tests C++ functions in the Miscallaneous category", function() end) it("turns saving on when told which command line, or none at all, but not whether to", function() - -- BUG: both forms are meant to default to turning saving on - the - -- implementation says so, and the branch that would read a second - -- argument after a name is unreachable without one. Both count their - -- arguments one too high, so they reach the type check and raise - -- instead. Left pending rather than pinning the raise as the contract. - pending("setSaveCommandHistory() and setSaveCommandHistory(name) raise instead of turning saving on") local original = getSaveCommandHistory() finally(function() setSaveCommandHistory(original) end) - setSaveCommandHistory(false) + -- turning it off first is what makes turning it on observable, so the + -- off state is asserted rather than assumed + assert.is_true(setSaveCommandHistory(false)) + assert.is_false((getSaveCommandHistory())) assert.is_true(setSaveCommandHistory()) assert.is_true((getSaveCommandHistory())) - setSaveCommandHistory(false) + assert.is_true(setSaveCommandHistory(false)) + assert.is_false((getSaveCommandHistory())) assert.is_true(setSaveCommandHistory("main")) assert.is_true((getSaveCommandHistory())) end) + it("turns saving on for the command line it is named, and no other", function() + -- "main" is also the name the no-argument form falls back to, so only + -- a second command line can tell "the name was read" from "the name + -- was dropped and main was used" + local commandLine = "mudlet-spec-save-history" + createCommandLine(commandLine, 10, 10, 120, 30) + local original = getSaveCommandHistory() + finally(function() + setSaveCommandHistory("main", original) + deleteCommandLine(commandLine) + end) + + assert.is_true(setSaveCommandHistory(commandLine, false)) + assert.is_true(setSaveCommandHistory("main", false)) + + assert.is_true(setSaveCommandHistory(commandLine)) + + assert.is_true((getSaveCommandHistory(commandLine))) + assert.is_false((getSaveCommandHistory("main"))) + end) + + it("returns nil+msg for a command line that does not exist", function() + local ok, err = setSaveCommandHistory("mudlet-spec-no-such-command-line") + assert.is_nil(ok) + assert.is_true(contains(err, "not found"), tostring(err)) + end) + it("round-trips through getSaveCommandHistory", function() local original = getSaveCommandHistory() finally(function() setSaveCommandHistory(original) end) diff --git a/src/mudlet-lua/tests/Package_spec.lua b/src/mudlet-lua/tests/Package_spec.lua index 4020e8909..ff62e290a 100644 --- a/src/mudlet-lua/tests/Package_spec.lua +++ b/src/mudlet-lua/tests/Package_spec.lua @@ -978,6 +978,24 @@ describe("Tests the functionality of verbosePackageInstall", function() assert.is_true(containsWrapped(text, "could not open file"), text) assert.is_false(packageInstalled("mudlet-spec-there-is-no-such-package")) end) + + it("names the file, not the whole path, when the install fails", function() + -- the announcement is trimmed on both branches, and only the success one is + -- reached from installPackageFromUrl's spec + local name = "mudlet-spec-there-is-no-such-package.mpackage" + -- an install asked for while a save is running is postponed and answered + -- with a bare true, so the failure under test would be announced a success + assert.is_true(waitForProfileSaveToPass(), "a profile save was still running") + local mark = getLastLineNumber("main") + + verbosePackageInstall(getMudletHomeDir() .. "/" .. name) + + local text = textFrom(mark) + assert.is_true(containsWrapped(text, "Installing '" .. name .. "' failed:"), text) + -- the reason installPackage() gives does name the whole path, so only the + -- announcement's own name is checked for having been trimmed + assert.is_false(containsWrapped(text, "Installing '" .. getMudletHomeDir()), text) + end) end) describe("Tests the functionality of verboseModuleInstall", function() @@ -1046,11 +1064,9 @@ describe("Tests the functionality of installPackageFromUrl", function() end) it("names the file, not the whole path, in the announcement", function() - -- BUG: verbosePackageInstall() strips the profile folder off the name it - -- announces, but uses that folder as a Lua pattern - a profile path holding - -- a "-" (a home folder with one will do it) never matches, so the whole - -- path is announced instead of the file. - pending("verbosePackageInstall() strips the profile folder with an unescaped Lua pattern") + -- this only bites while the profile name holds a Lua pattern magic + -- character ("Mudlet self-test" holds a "-"): a pattern-based strip finds + -- nothing to match there and announces the whole path defer(function() removeFixturePackage(minimalPackage) os.remove(getMudletHomeDir() .. "/" .. downloadedName)