hyprpm: Minor optimizations and refactor of helpers and progress bar (#10246)

* hyprpm: optimize sys.cpp

* hyprpm: refine progress bar logic

* chore: fix fetchSuperuserBins

* chore: modify one line if/else statements

* chore: fix if/else statements

* chore: follow naming convention for const vars

* chore: revert shell escape logic

hyprpm/src/helpers/Sys.cpp

@@ -1,59 +1,72 @@
 #include "Sys.hpp"
 #include "Die.hpp"
 #include "StringUtils.hpp"
+
 #include <pwd.h>
 #include <unistd.h>
 #include <print>
 #include <filesystem>
+#include <optional>
 
 #include <hyprutils/os/Process.hpp>
 #include <hyprutils/string/VarList.hpp>
+
 using namespace Hyprutils::OS;
 using namespace Hyprutils::String;
 
-static const std::vector<const char*> SUPERUSER_BINARIES = {
+inline constexpr std::array<std::string_view, 3> SUPERUSER_BINARIES = {
     "sudo",
     "doas",
     "run0",
 };
 
+static std::string fetchSuperuserBins() {
+    std::ostringstream oss;
+    auto               it = SUPERUSER_BINARIES.begin();
+    if (it != SUPERUSER_BINARIES.end()) {
+        oss << *it++;
+        for (; it != SUPERUSER_BINARIES.end(); ++it)
+            oss << ", " << *it;
+    }
+
+    return oss.str();
+}
+
 static bool executableExistsInPath(const std::string& exe) {
-    if (!getenv("PATH"))
+    const char* PATHENV = std::getenv("PATH");
+    if (!PATHENV)
         return false;
 
-    static CVarList paths(getenv("PATH"), 0, ':', true);
+    CVarList        paths(PATHENV, 0, ':', true);
+    std::error_code ec;
 
-    for (auto& p : paths) {
-        std::string     path = p + std::string{"/"} + exe;
-        std::error_code ec;
-        if (!std::filesystem::exists(path, ec) || ec)
+    for (const auto& PATH : paths) {
+        std::filesystem::path candidate = std::filesystem::path(PATH) / exe;
+        if (!std::filesystem::exists(candidate, ec) || ec)
             continue;
-
-        if (!std::filesystem::is_regular_file(path, ec) || ec)
+        if (!std::filesystem::is_regular_file(candidate, ec) || ec)
             continue;
-
-        auto stat = std::filesystem::status(path, ec);
+        auto perms = std::filesystem::status(candidate, ec).permissions();
         if (ec)
             continue;
-
-        auto perms = stat.permissions();
-
-        return std::filesystem::perms::none != (perms & std::filesystem::perms::others_exec);
+        if ((perms & std::filesystem::perms::others_exec) != std::filesystem::perms::none)
+            return true;
     }
 
     return false;
 }
 
-static std::pair<std::string, int> execAndGet(std::string cmd, bool noRedirect = false) {
+static std::optional<std::pair<std::string, int>> execAndGet(std::string_view cmd, bool noRedirect = false) {
+    std::string command = std::string{cmd};
     if (!noRedirect)
-        cmd += " 2>&1";
-
-    CProcess proc("/bin/sh", {"-c", cmd});
+        command += " 2>&1";
 
+    CProcess proc("/bin/sh", {"-c", command});
     if (!proc.runSync())
-        return {"error", 1};
+        // optional handles nullopt gracefully
+        return std::nullopt;
 
-    return {proc.stdOut(), proc.exitCode()};
+    return {{proc.stdOut(), proc.exitCode()}};
 }
 
 int NSys::getUID() {
@@ -69,23 +82,22 @@ int NSys::getEUID() {
 }
 
 bool NSys::isSuperuser() {
-    return getuid() != geteuid() || !geteuid();
+    return getuid() != geteuid() || geteuid() == 0;
 }
 
 std::string NSys::runAsSuperuser(const std::string& cmd) {
-    for (const auto& SB : SUPERUSER_BINARIES) {
-        if (!executableExistsInPath(SB))
+    for (const auto& BIN : SUPERUSER_BINARIES) {
+        if (!executableExistsInPath(std::string{BIN}))
             continue;
 
-        const auto RESULT = execAndGet(std::string{SB} + " /bin/sh -c \"" + cmd + "\"", true);
-
-        if (RESULT.second != 0)
+        const auto result = execAndGet(std::string{BIN} + " /bin/sh -c " + cmd, true);
+        if (!result.has_value() || result->second != 0)
             Debug::die("Failed to run a command as sudo. This could be due to an invalid password, or a hyprpm bug.");
 
-        return RESULT.first;
+        return result->first;
     }
 
-    Debug::die("Failed to find a superuser binary. Supported: sudo, doas, run0.");
+    Debug::die("{} {}", "Failed to find a superuser binary. Supported: ", fetchSuperuserBins());
     return "";
 }
 
@@ -96,15 +108,15 @@ void NSys::cacheSudo() {
 }
 
 void NSys::dropSudo() {
-    for (const auto& SB : SUPERUSER_BINARIES) {
-        if (!executableExistsInPath(SB))
+    for (const auto& BIN : SUPERUSER_BINARIES) {
+        if (!executableExistsInPath(std::string{BIN}))
             continue;
 
-        if (SB == std::string_view{"sudo"})
+        if (BIN == "sudo")
             execAndGet("sudo -k");
-        else
-            std::println("{}", infoString("Don't know how to drop timestamp for {}, ignoring.", SB));
-
-        return;
+        else {
+            // note the superuser binary that is being dropped
+            std::println("{}", infoString("Don't know how to drop timestamp for '{}', ignoring.", BIN));
+        }
     }
 }

hyprpm/src/main.cpp

@@ -4,7 +4,6 @@
 #include "core/DataState.hpp"
 #include "helpers/Sys.hpp"
 
-#include <cstdio>
 #include <vector>
 #include <string>
 #include <print>

hyprpm/src/progress/CProgressBar.cpp

@@ -1,82 +1,78 @@
 #include "CProgressBar.hpp"
 
 #include <sys/ioctl.h>
-#include <algorithm>
+#include <unistd.h>
 #include <cmath>
 #include <format>
-
 #include <print>
-#include <stdio.h>
-#include <unistd.h>
+#include <cstdio>
+
+#include <algorithm>
+#include <sstream>
 
 #include "../helpers/Colors.hpp"
 
-void CProgressBar::printMessageAbove(const std::string& msg) {
-    struct winsize w;
+static winsize getTerminalSize() {
+    winsize w{};
     ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
+    return w;
+}
 
-    std::string spaces;
-    spaces.reserve(w.ws_col);
-    for (size_t i = 0; i < w.ws_col; ++i) {
-        spaces += ' ';
-    }
+static void clearCurrentLine() {
+    std::print("\r\33[2K"); // ansi escape sequence to clear entire line
+}
 
-    std::println("\r{}\r{}", spaces, msg);
-    print();
+void CProgressBar::printMessageAbove(const std::string& msg) {
+    clearCurrentLine();
+    std::print("\r{}\n", msg);
+
+    print(); // reprint bar underneath
 }
 
 void CProgressBar::print() {
-    struct winsize w;
-    ioctl(STDOUT_FILENO, TIOCGWINSZ, &w);
+    const auto w = getTerminalSize();
 
-    if (m_bFirstPrint)
+    if (m_bFirstPrint) {
         std::print("\n");
-    m_bFirstPrint = false;
-
-    std::string spaces;
-    spaces.reserve(w.ws_col);
-    for (size_t i = 0; i < w.ws_col; ++i) {
-        spaces += ' ';
+        m_bFirstPrint = false;
     }
 
-    std::print("\r{}\r", spaces);
+    clearCurrentLine();
 
-    std::string message = "";
-
-    float       percentDone = 0;
-    if (m_fPercentage >= 0)
+    float percentDone = 0.0f;
+    if (m_fPercentage >= 0.0f)
         percentDone = m_fPercentage;
-    else
-        percentDone = (float)m_iSteps / (float)m_iMaxSteps;
-
-    const auto BARWIDTH = std::clamp(w.ws_col - static_cast<unsigned long>(m_szCurrentMessage.length()) - 2, 0UL, 50UL);
-
-    // draw bar
-    message += std::string{" "} + Colors::GREEN;
-    size_t i = 0;
-    for (; i < std::floor(percentDone * BARWIDTH); ++i) {
-        message += "━";
+    else {
+        // check for divide-by-zero
+        percentDone = m_iMaxSteps > 0 ? static_cast<float>(m_iSteps) / m_iMaxSteps : 0.0f;
     }
+    // clamp to ensure no overflows (sanity check)
+    percentDone = std::clamp(percentDone, 0.0f, 1.0f);
+
+    const size_t       BARWIDTH = std::clamp<size_t>(w.ws_col - m_szCurrentMessage.length() - 2, 0, 50);
+
+    std::ostringstream oss;
+    oss << ' ' << Colors::GREEN;
+
+    size_t filled = static_cast<size_t>(std::floor(percentDone * BARWIDTH));
+    size_t i      = 0;
+
+    for (; i < filled; ++i)
+        oss << "━";
 
     if (i < BARWIDTH) {
-        i++;
-
-        message += std::string{"╍"} + Colors::RESET;
-
-        for (; i < BARWIDTH; ++i) {
-            message += "━";
-        }
+        oss << "╍" << Colors::RESET;
+        ++i;
+        for (; i < BARWIDTH; ++i)
+            oss << "━";
     } else
-        message += Colors::RESET;
+        oss << Colors::RESET;
 
-    // draw progress
-    if (m_fPercentage >= 0)
-        message += "  " + std::format("{}%", static_cast<int>(percentDone * 100.0)) + " ";
+    if (m_fPercentage >= 0.0f)
+        oss << "  " << std::format("{}%", static_cast<int>(percentDone * 100.0)) << ' ';
     else
-        message += "  " + std::format("{} / {}", m_iSteps, m_iMaxSteps) + " ";
-
-    // draw message
-    std::print("{} {}", message, m_szCurrentMessage);
+        oss << "  " << std::format("{} / {}", m_iSteps, m_iMaxSteps) << ' ';
 
+    std::print("{} {}", oss.str(), m_szCurrentMessage);
     std::fflush(stdout);
 }