refactor(pager): follow common conventions + add more tests

Also, introduce `DWARFS_PAGER`. This avoids messing with `PAGER` by
randomly appending `-R`, but rather sets `LESS` options, unless set
by the user. Also, this now uses the standard execute-via-sh approach
on POSIX and only implements command parsing on Windows.

Author: mhx

CMakeLists.txt

@@ -821,9 +821,11 @@ if(WITH_TESTS)
       find_program(DWARFSEXTRACT_EXE NAMES dwarfsextract dwarfsextract.exe)
     endif()
     if(WITH_TOOLS OR (MKDWARFS_EXE AND DWARFSCK_EXE AND DWARFSEXTRACT_EXE))
-      list(APPEND DWARFS_TESTS
-        tools_test
-      )
+      list(APPEND DWARFS_TESTS tools_test)
+      if(WITH_MAN_OPTION)
+        list(APPEND DWARFS_TESTS pager_test)
+        add_executable(pager_test_helper test/pager_test_helper.cpp)
+      endif()
     endif()
   endif()
 

cmake/libdwarfs_tool.cmake

@@ -49,6 +49,11 @@ if(WITH_MAN_OPTION)
     tools/src/tool/pager.cpp
     tools/src/tool/render_manpage.cpp
   )
+  if(WIN32)
+    target_sources(dwarfs_tool PRIVATE
+      tools/src/tool/internal/pager_command_line.cpp
+    )
+  endif()
 endif()
 
 target_link_libraries(dwarfs_tool PUBLIC dwarfs_common Boost::program_options)

test/manpage_test.cpp

@@ -236,92 +236,3 @@ TEST_P(manpage_coverage_test, options) {
 
 INSTANTIATE_TEST_SUITE_P(dwarfs, manpage_coverage_test,
                          ::testing::ValuesIn(coverage_tests));
-
-TEST(pager_test, find_pager_program) {
-  auto resolver = [](std::filesystem::path const& name) {
-    std::map<std::string, std::filesystem::path> const programs = {
-        {"less", "/whatever/bin/less"},
-        {"more", "/somewhere/bin/more"},
-        {"cat", "/bin/cat"},
-    };
-    for (auto const& [n, p] : programs) {
-      if (name == n || name == p) {
-        return p;
-      }
-    }
-    return std::filesystem::path{};
-  };
-
-  {
-    test::os_access_mock os;
-    os.set_executable_resolver(
-        [](std::filesystem::path const&) { return std::filesystem::path{}; });
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_FALSE(pager);
-    }
-
-    os.set_executable_resolver(resolver);
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/whatever/bin/less", pager->name);
-      EXPECT_EQ(std::vector<std::string>({"-R"}), pager->args);
-    }
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/whatever/bin/less", pager->name);
-      EXPECT_EQ(std::vector<std::string>({"-R"}), pager->args);
-    }
-
-    os.set_access_fail("more");
-    os.set_access_fail("less");
-
-    os.setenv("PAGER", "more");
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/somewhere/bin/more", pager->name);
-      EXPECT_TRUE(pager->args.empty());
-    }
-
-    os.setenv("PAGER", "less");
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/whatever/bin/less", pager->name);
-      EXPECT_THAT(pager->args, ::testing::ElementsAre("-R"));
-    }
-
-    os.setenv("PAGER", "cat");
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_FALSE(pager);
-    }
-
-    os.setenv("PAGER", "/bla/foo");
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/bla/foo", pager->name);
-      EXPECT_TRUE(pager->args.empty());
-    }
-
-    os.setenv("PAGER", R"("/bla/foo")");
-
-    {
-      auto pager = find_pager_program(os);
-      ASSERT_TRUE(pager);
-      EXPECT_EQ("/bla/foo", pager->name);
-      EXPECT_TRUE(pager->args.empty());
-    }
-  }
-}