fix: three Lua API bugs the core specs found - command history, phantom profiles, package names (#9816)

#### Brief overview of PR changes/additions

- `setSaveCommandHistory()` and `setSaveCommandHistory(name)` turn
saving on instead of raising: the argument count was tested one too
high, which also made the branch reading a boolean after a name
unreachable.
- `setProfileInformation()` / `clearProfileInformation()` refuse a
profile that is not there instead of conjuring one.
`mudlet::writeProfileData()` creates whatever folder it is handed, so an
unknown name used to leave a phantom in `getProfiles()` and the
connection dialog. The check went into the two Lua entry points because
the connection dialog needs that folder creation to save a character
name for a default game (#8101), and a game Mudlet ships with only
counts once it has a folder of its own.
- `verbosePackageInstall()` names the package rather than its whole
path: the profile folder was stripped with `gsub`, which reads a path as
a Lua pattern, and the `-` in "Mudlet self-test" (or in any of the many
real paths holding one) never matches.

#### Motivation for adding to Mudlet

All three were found and filed while writing the core and package specs
merged in #9802, where they were left as `pending()`. Those specs are
flipped to real assertions here, so each fix is pinned by a test that
fails without it.

#### Test case

`lua setSaveCommandHistory()`, `lua setProfileInformation("no such
profile", "x")` then `lua display(getProfiles())`, and `lua
installPackage("https://.../thing.mpackage")` from a profile whose path
holds a `-`.

Closes #9803
Closes #9804
Closes #9806

Assisted-by: Claude:claude-opus-5

#### Other info

The red ubuntu and windows legs are the known development-side
`ProfileLifecycleTest` failure, not this PR: development @ e42bd1e28
fails the same `initTestCase()` assertion (run 31464046946). A dedicated
PR fixes it.

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<bool, QString> 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<bool, QString> 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<bool, QString> 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<bool, QString> 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);

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

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)

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)