From add213832135a834b7ec81a4cc7f3dc00ee5e7f0 Mon Sep 17 00:00:00 2001
From: Riyyi <riyyi3@gmail.com>
Date: Sun, 2 Feb 2025 22:07:51 +0100
Subject: [PATCH] Manager: Fix unit test, add XML comment testcase

---
 src/dotfile.cpp           | 17 ++++++--
 test/unit/testdotfile.cpp | 88 +++++++++++++++++++++++++++++++++++----
 2 files changed, 94 insertions(+), 11 deletions(-)

diff --git a/src/dotfile.cpp b/src/dotfile.cpp
index 4d8ff48..180908d 100644
--- a/src/dotfile.cpp
+++ b/src/dotfile.cpp
@@ -405,16 +405,27 @@ void Dotfile::selectivelyCommentOrUncomment(const std::string& path)
 
 		size_t lineLength = line.size();
 		size_t commentStart = line.find(commentCharacter, indentation);
-		size_t commentEnd = line.find(commentTerminationCharacter, commentStart);
-		bool hasComment = commentStart != std::string::npos && commentEnd != std::string::npos;
+		size_t commentEnd = line.rfind(commentTerminationCharacter);
+		bool hasComment = commentStart != std::string::npos && (commentTerminationCharacter.empty() || commentEnd != std::string::npos);
 
+		// Lines that have a comment at the *end* of the line aren't considered commented lines
+		if (hasComment && line.find_first_not_of(" \t") < commentStart) {
+			hasComment = false;
+		}
+
+		// Uncomment line
 		if (hasComment && !addComment) {
 			size_t contentStart = line.find_first_not_of(" \t", commentStart + commentCharacter.size());
 			size_t contentLength = commentEnd - contentStart;
 
+			// Trim trailing whitespace
+			std::string content = line.substr(contentStart, contentLength);
+			content = content.substr(0, content.find_last_not_of(" \t") + 1);
+
 			line = line.substr(0, indentation)
-			       + line.substr(contentStart, contentLength);
+			       + content;
 		}
+		// Comment line
 		else if (!hasComment && addComment) {
 			size_t contentStart = line.find_first_not_of(" \t");
 			size_t contentLength = line.size() - contentStart;
diff --git a/test/unit/testdotfile.cpp b/test/unit/testdotfile.cpp
index b87a04f..3742fa7 100644
--- a/test/unit/testdotfile.cpp
+++ b/test/unit/testdotfile.cpp
@@ -1,11 +1,10 @@
 /*
- * Copyright (C) 2022 Riyyi
+ * Copyright (C) 2022,2025 Riyyi
  *
  * SPDX-License-Identifier: MIT
  */
 
 #include <cstddef>    // size_t
-#include <cstdint>    // uint32_t
 #include <cstdio>     // stderr
 #include <filesystem> // path
 #include <string>
@@ -13,12 +12,12 @@
 #include <unordered_map>
 #include <vector>
 
+#include "ruc/file.h"
+
 #include "config.h"
 #include "dotfile.h"
 #include "machine.h"
 #include "macro.h"
-#include "ruc/file.h"
-#include "ruc/system.h"
 #include "testcase.h"
 #include "testsuite.h"
 
@@ -28,7 +27,7 @@ const size_t homeDirectorySize = homeDirectory.string().size();
 
 void createTestDotfiles(const std::vector<std::string>& fileNames, const std::vector<std::string>& fileContents, bool asRoot = false)
 {
-	EXPECT(fileNames.size() == fileContents.size(), return );
+	EXPECT(fileNames.size() == fileContents.size(), return);
 
 	if (root && !asRoot) {
 		setegid(Machine::the().gid());
@@ -503,7 +502,7 @@ TEST_CASE(PushDotfilesWithIgnorePattern)
 TEST_CASE(PushDotfilesSelectivelyComment)
 {
 	std::vector<std::string> fileNames;
-	for (size_t i = 0; i < 36; ++i) {
+	for (size_t i = 0; i < 44; ++i) {
 		fileNames.push_back("__test-file-" + std::to_string(i + 1));
 	}
 
@@ -678,6 +677,43 @@ test data /**/ comment
 	  /*	  test data /**/ uncomment	  */
 	  /* <<< */
 )",
+
+		// Comment <!---->
+		"<!-- >>> distro=" + placeholder + " hostname=" + hostname + " user=" + username + " -->" + R"(
+test data <!----> comment
+<!-- <<< -->
+)",
+		"  <!-- >>> distro=" + distro + " hostname=" + placeholder + " user=" + username + " -->" + R"(
+  test data <!----> comment
+  <!-- <<< -->
+)",
+		"	<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + placeholder + " -->" + R"(
+	test data <!----> comment
+	<!-- <<< -->
+)",
+		"	  <!-- >>> distro=" + placeholder + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	  test data <!----> comment
+	  <!-- <<< -->
+)",
+
+		// Uncomment <!---->
+		"<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+<!-- test data <!----> uncomment -->
+<!-- <<< -->
+)",
+		"  <!-- >>> distro="
+			+ distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+  <!--  test data <!----> uncomment  -->
+  <!-- <<< -->
+)",
+		"	<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	<!--	test data <!----> uncomment	-->
+	<!-- <<< -->
+)",
+		"	  <!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	  <!--	  test data <!----> uncomment	  -->
+	  <!-- <<< -->
+)",
 	};
 
 	std::vector<std::string> pushedFileContents = {
@@ -846,6 +882,42 @@ test data /**/ uncomment
 	  test data /**/ uncomment
 	  /* <<< */
 )",
+
+		// Comment <!---->
+		"<!-- >>> distro=" + placeholder + " hostname=" + hostname + " user=" + username + " -->" + R"(
+<!-- test data <!----> comment -->
+<!-- <<< -->
+)",
+		"  <!-- >>> distro=" + distro + " hostname=" + placeholder + " user=" + username + " -->" + R"(
+  <!-- test data <!----> comment -->
+  <!-- <<< -->
+)",
+		"	<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + placeholder + " -->" + R"(
+	<!-- test data <!----> comment -->
+	<!-- <<< -->
+)",
+		"	  <!-- >>> distro=" + placeholder + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	  <!-- test data <!----> comment -->
+	  <!-- <<< -->
+)",
+
+		// Uncomment <!---->
+		"<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+test data <!----> uncomment
+<!-- <<< -->
+)",
+		"  <!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+  test data <!----> uncomment
+  <!-- <<< -->
+)",
+		"	<!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	test data <!----> uncomment
+	<!-- <<< -->
+)",
+		"	  <!-- >>> distro=" + distro + " hostname=" + hostname + " user=" + username + " -->" + R"(
+	  test data <!----> uncomment
+	  <!-- <<< -->
+)",
 	};
 
 	createTestDotfiles(fileNames, fileContents);
@@ -866,7 +938,7 @@ test data /**/ uncomment
 
 TEST_CASE(AddSystemDotfiles)
 {
-	EXPECT(geteuid() == 0, return );
+	EXPECT(geteuid() == 0, return);
 
 	Config::the().setSystemPatterns({ "/etc/", "/usr/lib/" });
 	Dotfile::the().add({ "/etc/group", "/usr/lib/os-release" });
@@ -881,7 +953,7 @@ TEST_CASE(AddSystemDotfiles)
 
 TEST_CASE(PullSystemDotfiles)
 {
-	EXPECT(geteuid() == 0, return );
+	EXPECT(geteuid() == 0, return);
 
 	createTestDotfiles({ "etc/group" }, { "" }, true);