diff --git a/.gitignore b/.gitignore index 2cd8b97..1a5db3a 100644 --- a/.gitignore +++ b/.gitignore @@ -366,6 +366,7 @@ tests/*.exe tests/*.obj tests/test_config.json tests/test_config_reload.json +tests/migrate_*.json # Rotation-test output: the harness rewrites these on every run, and the # .1/.2 suffixes fall outside the *.log rule above. tests/rotation_test.log.* diff --git a/VerseLinkWindows/ConfigManager.cpp b/VerseLinkWindows/ConfigManager.cpp index 8bedc72..fb0f6ee 100644 --- a/VerseLinkWindows/ConfigManager.cpp +++ b/VerseLinkWindows/ConfigManager.cpp @@ -1,10 +1,39 @@ #include "ConfigManager.h" #include "Logger.h" +#include "StringExtensions.h" #include #include #include #include #include +#include + +// Declared here rather than on the link line so this file stays self-contained: +// the test harness compiles it with a bare cl invocation that has no library +// list of its own. SHGetKnownFolderPath needs shell32, CoTaskMemFree ole32. +#pragma comment(lib, "shell32.lib") +#pragma comment(lib, "ole32.lib") + +namespace { + // Config paths are UTF-8 like every other string here, but MSVC's narrow + // fstream constructors interpret char* paths in the ANSI codepage. Going + // through a wide std::filesystem::path keeps a user whose profile name is + // not representable in that codepage working. + std::filesystem::path AsPath(const std::string& utf8Path) { + return std::filesystem::path(StringExtensions::Utf8ToWide(utf8Path)); + } + + std::string ExecutableDirectory() { + wchar_t buffer[MAX_PATH] = {}; + const DWORD length = GetModuleFileNameW(nullptr, buffer, MAX_PATH); + if (length == 0 || length >= MAX_PATH) return ""; + + std::wstring path(buffer, length); + const size_t slash = path.find_last_of(L"\\/"); + if (slash == std::wstring::npos) return ""; + return StringExtensions::WideToUtf8(path.substr(0, slash)); + } +} // Static member definitions std::unique_ptr ConfigManager::instance = nullptr; @@ -32,6 +61,67 @@ void ConfigManager::setDefaults() { config = VerseLinkConfig(); // Use default values from struct } +std::string ConfigManager::userConfigPath() { + PWSTR roaming = nullptr; + if (FAILED(SHGetKnownFolderPath(FOLDERID_RoamingAppData, 0, nullptr, &roaming)) || !roaming) { + if (roaming) CoTaskMemFree(roaming); + return ""; + } + + std::filesystem::path directory(roaming); + CoTaskMemFree(roaming); + directory /= L"VerseLink"; + + std::error_code ec; + std::filesystem::create_directories(directory, ec); + if (ec) { + return ""; + } + + return StringExtensions::WideToUtf8((directory / L"config.json").wstring()); +} + +std::vector ConfigManager::legacyConfigPaths() { + std::vector candidates; + + // Beside the exe first: that is where an unzipped copy kept its settings. + const std::string exeDir = ExecutableDirectory(); + if (!exeDir.empty()) { + candidates.push_back(exeDir + "\\config.json"); + } + + // Then the working directory, which is what the app used to load from. + candidates.push_back("config.json"); + return candidates; +} + +std::string ConfigManager::migrateLegacyConfig(const std::string& targetPath, + const std::vector& candidates) { + if (targetPath.empty()) return ""; + + std::error_code ec; + const std::filesystem::path target = AsPath(targetPath); + if (std::filesystem::exists(target, ec)) { + return ""; // never overwrite settings that are already in place + } + + for (const auto& candidate : candidates) { + if (candidate.empty()) continue; + + const std::filesystem::path source = AsPath(candidate); + if (std::filesystem::equivalent(source, target, ec)) continue; + if (!std::filesystem::is_regular_file(source, ec)) continue; + + std::filesystem::copy_file(source, target, + std::filesystem::copy_options::overwrite_existing, ec); + if (!ec) { + return candidate; + } + } + + return ""; +} + std::string ConfigManager::expandPath(const std::string& path) { // For now, just return the path as-is // Could be expanded to handle environment variables, relative paths, etc. @@ -114,7 +204,7 @@ bool ConfigManager::load() { return false; } - std::ifstream file(configFilePath); + std::ifstream file(AsPath(configFilePath)); if (!file.is_open()) { LOG_WARNING("Config file not found: " + configFilePath + ", creating with defaults"); save(); // Create default config file @@ -196,7 +286,7 @@ bool ConfigManager::save() { } const VerseLinkConfig& config = snapshot; - std::ofstream file(configFilePath); + std::ofstream file(AsPath(configFilePath)); if (!file.is_open()) { LOG_ERROR("Failed to open config file for writing: " + configFilePath); return false; diff --git a/VerseLinkWindows/ConfigManager.h b/VerseLinkWindows/ConfigManager.h index 8655a27..5deabdd 100644 --- a/VerseLinkWindows/ConfigManager.h +++ b/VerseLinkWindows/ConfigManager.h @@ -6,6 +6,7 @@ #include #include #include +#include #include struct VerseLinkConfig { @@ -78,6 +79,28 @@ class ConfigManager { public: static ConfigManager& getInstance(); static void initialize(const std::string& configFilePath = "config.json"); + + // Where an installed run keeps its settings: %APPDATA%\VerseLink\config.json, + // creating the directory if needed. Returns "" if the folder cannot be found. + // + // The install directory is not a safe place for this file. The app runs + // AsInvoker, so it cannot write under Program Files - saves would fail, or be + // silently redirected into VirtualStore where the app then reads a different + // file than the one on disk. A CWD-relative "config.json" is worse still: + // launched from a Start Menu shortcut the working directory is not the + // install folder, so settings would neither load nor save where expected. + static std::string userConfigPath(); + + // Config files an older version may have left beside the exe or in the + // working directory, in the order they should be preferred. + static std::vector legacyConfigPaths(); + + // Copies the first candidate that exists to targetPath, but only when + // targetPath does not exist yet, so an upgrading user keeps their settings + // and a later run never overwrites them. Returns the path migrated from, or + // "" when nothing was migrated. Paths are UTF-8. + static std::string migrateLegacyConfig(const std::string& targetPath, + const std::vector& candidates); bool load(); bool save(); diff --git a/VerseLinkWindows/SystemTray.cpp b/VerseLinkWindows/SystemTray.cpp index c3806d3..b60e212 100644 --- a/VerseLinkWindows/SystemTray.cpp +++ b/VerseLinkWindows/SystemTray.cpp @@ -3,13 +3,26 @@ #include "Logger.h" #include "SettingsDialog.h" #include "StringExtensions.h" -#include "VerseLinkWindows.h" // DescribeHotkey +#include "VerseLinkWindows.h" // DescribeHotkey, ResolveLogPath +#include "Version.h" #include #include #include #pragma comment(lib, "comctl32.lib") +namespace { + // The icon compiled into the exe, falling back to the generic application + // icon if the resource is missing. Before VerseLink.rc existed the exe had + // no icon at all, so the tray showed the default executable glyph. + HICON DefaultAppIcon() { + if (HICON icon = LoadIconW(GetModuleHandleW(nullptr), MAKEINTRESOURCEW(IDI_VERSELINK))) { + return icon; + } + return LoadIcon(nullptr, IDI_APPLICATION); + } +} + SystemTray::SystemTray(HWND parentHwnd) : hwnd(parentHwnd), isVisible(false), hCustomIcon(nullptr) { ZeroMemory(&nid, sizeof(NOTIFYICONDATA)); hPopupMenu = nullptr; @@ -35,7 +48,7 @@ bool SystemTray::Initialize() { nid.uID = 1; nid.uFlags = NIF_ICON | NIF_MESSAGE | NIF_TIP; nid.uCallbackMessage = WM_TRAYICON; - nid.hIcon = hCustomIcon ? hCustomIcon : LoadIcon(nullptr, IDI_APPLICATION); + nid.hIcon = hCustomIcon ? hCustomIcon : DefaultAppIcon(); wcscpy_s(nid.szTip, sizeof(nid.szTip)/sizeof(WCHAR), L"VerseLink"); return true; @@ -173,7 +186,7 @@ void SystemTray::ShowAboutDialog() { // ASCII-only; literal Unicode here has been corrupted by a bad round-trip // before, and CI now rejects non-ASCII bytes in sources. const std::wstring aboutText = - L"VerseLink v1.0\n\n" + L"VerseLink " VERSELINK_VERSION_WIDE L"\n\n" L"A Bible verse lookup and insertion tool.\n\n" L"Features:\n" L"\u2022 Hotkey-activated verse lookup (" + hotkey + L")\n" @@ -192,7 +205,7 @@ void SystemTray::ShowLogDialog() { // Read log file content std::string logContent; auto& config = ConfigManager::getInstance(); - std::string logPath = config.getLogFilePath(); + std::string logPath = ResolveLogPath(config.getLogFilePath()); std::ifstream logFile(logPath); if (logFile.is_open()) { @@ -308,7 +321,7 @@ void SystemTray::SetCustomIcon(const std::string& iconPath) { } } else { // Fall back to default icon if custom failed - nid.hIcon = LoadIcon(nullptr, IDI_APPLICATION); + nid.hIcon = DefaultAppIcon(); if (isVisible) { if (Shell_NotifyIcon(NIM_MODIFY, &nid)) { LogMessage("Reverted to default system tray icon"); diff --git a/VerseLinkWindows/VerseLink.rc b/VerseLinkWindows/VerseLink.rc new file mode 100644 index 0000000..c4cc854 --- /dev/null +++ b/VerseLinkWindows/VerseLink.rc @@ -0,0 +1,39 @@ +#include +#include "Version.h" + +// Embedded application icon. Without this the exe has no icon of its own, so +// Explorer, the taskbar and any installer-created shortcut fall back to the +// generic executable icon. +IDI_VERSELINK ICON "VerseLinkIcon.ico" + +VS_VERSION_INFO VERSIONINFO + FILEVERSION VERSELINK_VERSION_MAJOR,VERSELINK_VERSION_MINOR,VERSELINK_VERSION_PATCH,0 + PRODUCTVERSION VERSELINK_VERSION_MAJOR,VERSELINK_VERSION_MINOR,VERSELINK_VERSION_PATCH,0 + FILEFLAGSMASK VS_FFI_FILEFLAGSMASK +#ifdef _DEBUG + FILEFLAGS VS_FF_DEBUG +#else + FILEFLAGS 0x0L +#endif + FILEOS VOS_NT_WINDOWS32 + FILETYPE VFT_APP + FILESUBTYPE VFT2_UNKNOWN +BEGIN + BLOCK "StringFileInfo" + BEGIN + BLOCK "040904b0" // U.S. English, Unicode + BEGIN + VALUE "CompanyName", "sethdtwigg" + VALUE "FileDescription", "VerseLink - Bible verse lookup and insertion" + VALUE "FileVersion", VERSELINK_VERSION_STRING + VALUE "InternalName", "VerseLinkWindows" + VALUE "OriginalFilename", "VerseLinkWindows.exe" + VALUE "ProductName", "VerseLink" + VALUE "ProductVersion", VERSELINK_VERSION_STRING + END + END + BLOCK "VarFileInfo" + BEGIN + VALUE "Translation", 0x409, 1200 + END +END diff --git a/VerseLinkWindows/VerseLinkWindows.cpp b/VerseLinkWindows/VerseLinkWindows.cpp index ed83eaa..5d9f9cf 100644 --- a/VerseLinkWindows/VerseLinkWindows.cpp +++ b/VerseLinkWindows/VerseLinkWindows.cpp @@ -5,9 +5,11 @@ #include "VerseFormatter.h" #include "SelfTest.h" #include "StringExtensions.h" +#include "Version.h" #include #include #include +#include #include // Global variable definitions @@ -44,11 +46,34 @@ static std::atomic g_shouldExit(false); static DWORD g_mainThreadId = 0; static HWND g_mainWindow = nullptr; +// Directory holding config.json; relative log paths resolve against it so an +// installed copy writes its log beside its settings rather than into whatever +// the working directory happens to be. +static std::string g_configDirectory; + // Hotkey registration state (for live re-registration when settings change) static bool g_hotkeyRegistered = false; static int g_registeredModifiers = 0; static int g_registeredVirtualKey = 0; +// An absolute logFilePath is honoured as-is; a relative one lands beside +// config.json. Left relative it would follow the working directory, so a +// shortcut-launched copy would scatter logs and the tray's "View Log" would +// read a different file than the one being written. +std::string ResolveLogPath(const std::string& configuredPath) { + if (configuredPath.empty() || g_configDirectory.empty()) { + return configuredPath; + } + + const std::filesystem::path path(StringExtensions::Utf8ToWide(configuredPath)); + if (path.is_absolute()) { + return configuredPath; + } + + const std::filesystem::path base(StringExtensions::Utf8ToWide(g_configDirectory)); + return StringExtensions::WideToUtf8((base / path).wstring()); +} + std::string DescribeHotkey(int modifiers, int virtualKey) { std::string description; if (modifiers & MOD_CONTROL) description += "Ctrl+"; @@ -153,7 +178,7 @@ void OnSettingsChanged() { UpdateTrayTooltip(); // Apply logger changes (level/outputs); reopen file if its path changed - Logger::initialize(config.logFilePath, + Logger::initialize(ResolveLogPath(config.logFilePath), static_cast(config.logLevel), config.enableConsoleLogging, config.enableFileLogging); @@ -275,8 +300,25 @@ bool RunVerseLink(HWND hwnd, SystemTray* systemTray) { bool GetConfiguration() { try { + // Settings live in %APPDATA%\VerseLink so an installed copy can write + // them. Only fall back to the working directory if that folder is + // unavailable, which is the old behaviour. + std::string configPath = ConfigManager::userConfigPath(); + std::string migratedFrom; + if (configPath.empty()) { + configPath = "config.json"; + } else { + // First run after an upgrade: carry settings over from the copy an + // older version kept beside the exe or in the working directory. + migratedFrom = ConfigManager::migrateLegacyConfig( + configPath, ConfigManager::legacyConfigPaths()); + } + + g_configDirectory = StringExtensions::WideToUtf8( + std::filesystem::path(StringExtensions::Utf8ToWide(configPath)).parent_path().wstring()); + // Initialize configuration manager - ConfigManager::initialize("config.json"); + ConfigManager::initialize(configPath); // Register settings change callback ConfigManager::getInstance().setSettingsChangeCallback(OnSettingsChanged); @@ -290,10 +332,13 @@ bool GetConfiguration() { // Initialize logger with config settings LogLevel logLevel = static_cast(config.logLevel); - Logger::initialize(config.logFilePath, logLevel, + Logger::initialize(ResolveLogPath(config.logFilePath), logLevel, config.enableConsoleLogging, config.enableFileLogging); - LOG_INFO("Configuration loaded from config.json"); + LOG_INFO("Configuration loaded from " + configPath); + if (!migratedFrom.empty()) { + LOG_INFO("Migrated settings from a previous installation at " + migratedFrom); + } return true; } catch (const std::exception& e) { @@ -374,11 +419,26 @@ int main() return 2; } + // Held for the life of the process. Two jobs: + // - the installer names this mutex so it can tell VerseLink is running and + // ask the user to close it, instead of failing on a locked exe mid-upgrade; + // - it keeps a second copy from starting, which would otherwise add a second + // tray icon and fight over the same hotkey. + // Named without a namespace prefix so it lives in the caller's session, which + // is what a per-user install wants and what the installer looks for. + HANDLE instanceMutex = CreateMutexW(nullptr, FALSE, VERSELINK_INSTANCE_MUTEX); + if (instanceMutex && GetLastError() == ERROR_ALREADY_EXISTS) { + std::cerr << "VerseLink is already running (check the system tray)." << std::endl; + CloseHandle(instanceMutex); + return 0; + } + g_mainThreadId = GetCurrentThreadId(); // Load configuration first (this also initializes the logger) if (!GetConfiguration()) { std::cerr << "Failed to load configuration, exiting..." << std::endl; + if (instanceMutex) CloseHandle(instanceMutex); return 1; } @@ -393,6 +453,7 @@ int main() if (!RegisterClass(&wc)) { LOG_ERROR("Failed to register window class"); + if (instanceMutex) CloseHandle(instanceMutex); return 1; } @@ -408,6 +469,7 @@ int main() if (!hwnd) { LOG_ERROR("Failed to create window"); + if (instanceMutex) CloseHandle(instanceMutex); return 1; } g_mainWindow = hwnd; @@ -453,6 +515,7 @@ int main() // path has no joinable std::thread to trip over on the way out. if (!RegisterAppHotkey(config.hotkeyModifiers, config.hotkeyVirtualKey)) { LOG_ERROR("Failed to register hotkey"); + if (instanceMutex) CloseHandle(instanceMutex); return 1; } UpdateTrayTooltip(); @@ -474,6 +537,8 @@ int main() ShutdownWorker(); + if (instanceMutex) CloseHandle(instanceMutex); + LOG_INFO("VerseLink shutdown complete"); return success ? 0 : 1; } diff --git a/VerseLinkWindows/VerseLinkWindows.h b/VerseLinkWindows/VerseLinkWindows.h index 9501623..cde175d 100644 --- a/VerseLinkWindows/VerseLinkWindows.h +++ b/VerseLinkWindows/VerseLinkWindows.h @@ -36,6 +36,12 @@ void QueueHotkeyTask(); // text reflects the configured combination instead of a hardcoded one. std::string DescribeHotkey(int modifiers, int virtualKey); +// Turns a configured logFilePath into the path actually being written: absolute +// paths as-is, relative ones resolved beside config.json. Anything that reads +// the log must go through this, or it will read a different file than the one +// the logger is writing. +std::string ResolveLogPath(const std::string& configuredPath); + // Function declarations void VerseLinkTask(); bool RunVerseLink(HWND hwnd, SystemTray* systemTray); diff --git a/VerseLinkWindows/VerseLinkWindows.vcxproj b/VerseLinkWindows/VerseLinkWindows.vcxproj index 3a6b547..06fd087 100644 --- a/VerseLinkWindows/VerseLinkWindows.vcxproj +++ b/VerseLinkWindows/VerseLinkWindows.vcxproj @@ -155,6 +155,9 @@ + + + @@ -167,6 +170,7 @@ + diff --git a/VerseLinkWindows/Version.h b/VerseLinkWindows/Version.h new file mode 100644 index 0000000..60c62ea --- /dev/null +++ b/VerseLinkWindows/Version.h @@ -0,0 +1,35 @@ +#pragma once +#ifndef Version_H +#define Version_H + +// Single source of version truth. Consumed by: +// - VerseLink.rc -> the exe's VERSIONINFO resource (Explorer, Add/Remove +// Programs, and what the installer reports) +// - SystemTray.cpp -> the About dialog +// - packaging/*.ps1 -> the installer's AppVersion, parsed from this file +// +// Bump VERSELINK_VERSION_STRING and the three numbers together. The installer +// compares versions to decide whether an install is an upgrade, so a release +// must never reuse a version that has already shipped. +#define VERSELINK_VERSION_MAJOR 1 +#define VERSELINK_VERSION_MINOR 1 +#define VERSELINK_VERSION_PATCH 0 +#define VERSELINK_VERSION_STRING "1.1.0" + +// Wide form for Win32 UI text, derived rather than duplicated so the two can +// never drift apart. +#define VERSELINK_WIDEN_(x) L ## x +#define VERSELINK_WIDEN(x) VERSELINK_WIDEN_(x) +#define VERSELINK_VERSION_WIDE VERSELINK_WIDEN(VERSELINK_VERSION_STRING) + +// Name of the single-instance mutex the app holds while running. The installer +// declares the same name in its AppMutex setting so it can detect a running +// copy and ask the user to close it, rather than failing on a locked exe part +// way through an upgrade. Changing this breaks that handshake with every +// installer already in the wild, so leave it alone. +#define VERSELINK_INSTANCE_MUTEX L"VerseLinkWindows.SingleInstance" + +// Resource identifiers +#define IDI_VERSELINK 101 + +#endif diff --git a/tests/test_harness.cpp b/tests/test_harness.cpp index 2703d0e..d70dabd 100644 --- a/tests/test_harness.cpp +++ b/tests/test_harness.cpp @@ -12,6 +12,7 @@ #include "StringExtensions.h" #include "TaskQueue.h" #include "VerseFormatter.h" +#include "Version.h" #include "tinyxml2.h" #include @@ -921,6 +922,88 @@ static void TestLoggerConcurrency() { Logger::initialize("verselink.log", Info, true, true); } +// --------------------------------------------------------------------------- +// Where settings live, and carrying them across an upgrade +// --------------------------------------------------------------------------- + +static void TestConfigLocation() { + BeginSection("Config location and migration"); + namespace fs = std::filesystem; + + // The version string is parsed by the packaging script while the .rc uses + // the numeric components; drift between them would ship an installer whose + // version disagrees with the exe it installs. + const std::string composed = std::to_string(VERSELINK_VERSION_MAJOR) + "." + + std::to_string(VERSELINK_VERSION_MINOR) + "." + + std::to_string(VERSELINK_VERSION_PATCH); + CHECK(std::string(VERSELINK_VERSION_STRING) == composed, + "VERSELINK_VERSION_STRING (" VERSELINK_VERSION_STRING ") matches the numeric " + "components (" + composed + ")"); + + // Settings must not live in the install directory: the app runs AsInvoker, + // so under Program Files a save would fail or be redirected to VirtualStore. + const std::string userPath = ConfigManager::userConfigPath(); + CHECK(!userPath.empty(), "userConfigPath() resolves"); + if (!userPath.empty()) { + CHECK(userPath.find("VerseLink") != std::string::npos, + "user config path sits under a VerseLink folder, got '" + userPath + "'"); + CHECK(userPath.size() > 11 && userPath.compare(userPath.size() - 11, 11, "config.json") == 0, + "user config path ends in config.json, got '" + userPath + "'"); + CHECK(fs::exists(fs::path(StringExtensions::Utf8ToWide(userPath)).parent_path()), + "userConfigPath() created its directory"); + } + + const auto legacy = ConfigManager::legacyConfigPaths(); + CHECK(!legacy.empty(), "legacy candidates are offered"); + CHECK(std::find(legacy.begin(), legacy.end(), std::string("config.json")) != legacy.end(), + "the working-directory config is a migration candidate"); + + const std::string source = "tests/migrate_source.json"; + const std::string target = "tests/migrate_target.json"; + const std::string absent = "tests/migrate_absent.json"; + for (const auto& path : { source, target, absent }) { + std::error_code ec; + fs::remove(path, ec); + } + { std::ofstream f(source); f << "{ \"bibleVersion\": \"NASB.xml\" }\n"; } + + // Upgrading: settings are carried over from the first candidate that exists. + std::string from = ConfigManager::migrateLegacyConfig(target, { absent, source }); + CHECK(from == source, "migrates from the first candidate that exists, got '" + from + "'"); + CHECK(fs::exists(target), "the target config was created"); + + // Re-running must never clobber settings already in place. + { std::ofstream f(target); f << "{ \"bibleVersion\": \"ESV.xml\" }\n"; } + from = ConfigManager::migrateLegacyConfig(target, { source }); + CHECK(from.empty(), "an existing target is never overwritten"); + { + std::ifstream f(target); + std::stringstream buffer; + buffer << f.rdbuf(); + CHECK(buffer.str().find("ESV.xml") != std::string::npos, + "the existing target's contents survive a second migration attempt"); + } + + // Nothing to migrate: no file is invented. + { std::error_code ec; fs::remove(target, ec); } + from = ConfigManager::migrateLegacyConfig(target, { absent }); + CHECK(from.empty(), "no candidate means no migration"); + CHECK(!fs::exists(target), "nothing is created when there is nothing to migrate"); + + // Degenerate inputs are refused rather than crashing. + CHECK(ConfigManager::migrateLegacyConfig("", { source }).empty(), + "an empty target path migrates nothing"); + CHECK(ConfigManager::migrateLegacyConfig(target, {}).empty(), + "an empty candidate list migrates nothing"); + CHECK(ConfigManager::migrateLegacyConfig(target, { source, source }).size() > 0, + "a real candidate still migrates after the degenerate cases"); + + for (const auto& path : { source, target }) { + std::error_code ec; + fs::remove(path, ec); + } +} + int main() { TestParsing(); TestDashVariants(); @@ -934,6 +1017,7 @@ int main() { TestNonReferenceInput(); TestTaskQueue(); TestLoggerConcurrency(); + TestConfigLocation(); TestConfigRoundTrip(); TestConfigConcurrency(); TestLogRotation();