diff --git a/include/dwarfs/os_access.h b/include/dwarfs/os_access.h index ce69c8aa..fd3d86be 100644 --- a/include/dwarfs/os_access.h +++ b/include/dwarfs/os_access.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -65,5 +66,7 @@ class os_access { std::error_code& ec) const = 0; virtual std::chrono::nanoseconds thread_get_cpu_time(std::thread::id tid, std::error_code& ec) const = 0; + virtual std::filesystem::path + find_executable(std::filesystem::path const& name) const = 0; }; } // namespace dwarfs diff --git a/include/dwarfs/os_access_generic.h b/include/dwarfs/os_access_generic.h index fa72bbd2..8a94f070 100644 --- a/include/dwarfs/os_access_generic.h +++ b/include/dwarfs/os_access_generic.h @@ -53,5 +53,7 @@ class os_access_generic : public os_access { std::error_code& ec) const override; std::chrono::nanoseconds thread_get_cpu_time(std::thread::id tid, std::error_code& ec) const override; + std::filesystem::path + find_executable(std::filesystem::path const& name) const override; }; } // namespace dwarfs diff --git a/include/dwarfs/pager.h b/include/dwarfs/pager.h index 44f5e604..5028ee1b 100644 --- a/include/dwarfs/pager.h +++ b/include/dwarfs/pager.h @@ -21,10 +21,21 @@ #pragma once +#include +#include #include +#include namespace dwarfs { -bool show_in_pager(std::string text); +class os_access; + +struct pager_program { + std::filesystem::path name; + std::vector args; +}; + +std::optional find_pager_program(os_access const& os); +void show_in_pager(pager_program const& pager, std::string text); } // namespace dwarfs diff --git a/src/dwarfs/os_access_generic.cpp b/src/dwarfs/os_access_generic.cpp index f067b7bc..f6ac5a53 100644 --- a/src/dwarfs/os_access_generic.cpp +++ b/src/dwarfs/os_access_generic.cpp @@ -25,6 +25,8 @@ #include #include +#include + #include "dwarfs/mmap.h" #include "dwarfs/os_access_generic.h" #include "dwarfs/util.h" @@ -193,4 +195,9 @@ os_access_generic::thread_get_cpu_time(std::thread::id tid, #endif } +std::filesystem::path +os_access_generic::find_executable(std::filesystem::path const& name) const { + return boost::process::search_path(name.wstring()).wstring(); +} + } // namespace dwarfs diff --git a/src/dwarfs/pager.cpp b/src/dwarfs/pager.cpp index a7bb0c85..33575c4d 100644 --- a/src/dwarfs/pager.cpp +++ b/src/dwarfs/pager.cpp @@ -26,6 +26,9 @@ #include #include +#include + +#include "dwarfs/os_access.h" #include "dwarfs/pager.h" namespace dwarfs { @@ -34,63 +37,53 @@ namespace { namespace bp = boost::process; -struct pager_def { - std::string name; - std::vector args; -}; - -std::vector const pagers{ +std::vector const pagers{ {"less", {"-R"}}, }; -auto find_executable(std::string name) { return bp::search_path(name); } +} // namespace + +std::optional find_pager_program(os_access const& os) { + if (auto pager_env = os.getenv("PAGER")) { + std::string_view sv{pager_env.value()}; + + if (sv == "cat") { + return std::nullopt; + } -std::pair> find_pager() { - if (auto pager_env = std::getenv("PAGER")) { - std::string_view sv(pager_env); if (sv.starts_with('"') && sv.ends_with('"')) { sv.remove_prefix(1); sv.remove_suffix(1); } - if (sv == "cat") { - return {}; + + std::filesystem::path p{std::string(sv)}; + + if (os.access(p, X_OK) == 0) { + return pager_program{p, {}}; } - boost::filesystem::path p{std::string(sv)}; - if (boost::filesystem::exists(p)) { - return {p.string(), {}}; - } - if (auto exe = find_executable(pager_env); !exe.empty()) { - return {exe, {}}; + + if (auto exe = os.find_executable(p); !exe.empty()) { + return pager_program{exe, {}}; } } for (auto const& p : pagers) { - if (auto exe = find_executable(std::string(p.name)); !exe.empty()) { - return {exe, p.args}; + if (auto exe = os.find_executable(p.name); !exe.empty()) { + return pager_program{exe, p.args}; } } - return {}; + return std::nullopt; } -} // namespace - -bool show_in_pager(std::string text) { - auto [pager_exe, pager_args] = find_pager(); - - if (pager_exe.empty()) { - return false; - } - +void show_in_pager(pager_program const& pager, std::string text) { boost::asio::io_service ios; - bp::child proc(pager_exe, bp::args(pager_args), + bp::child proc(pager.name.wstring(), bp::args(pager.args), bp::std_in = boost::asio::const_buffer(text.data(), text.size()), bp::std_out > stdout, ios); ios.run(); proc.wait(); - - return true; } } // namespace dwarfs diff --git a/src/dwarfs/tool.cpp b/src/dwarfs/tool.cpp index a8a45a96..ec85783f 100644 --- a/src/dwarfs/tool.cpp +++ b/src/dwarfs/tool.cpp @@ -91,11 +91,18 @@ void add_common_options(po::options_description& opts, #ifdef DWARFS_BUILTIN_MANPAGE void show_manpage(manpage::document doc, iolayer const& iol) { bool is_tty = iol.term->is_tty(iol.out); + auto content = render_manpage(doc, iol.term->width(), is_tty && iol.term->is_fancy()); - if (!is_tty || !show_in_pager(content)) { - iol.out << content; + + if (is_tty) { + if (auto pager = find_pager_program(*iol.os)) { + show_in_pager(*pager, content); + return; + } } + + iol.out << content; } #endif diff --git a/test/manpage_test.cpp b/test/manpage_test.cpp index 17a31c3b..6a54507e 100644 --- a/test/manpage_test.cpp +++ b/test/manpage_test.cpp @@ -25,8 +25,11 @@ #include #include +#include "dwarfs/pager.h" #include "dwarfs/render_manpage.h" +#include "test_helpers.h" + using namespace dwarfs; namespace { @@ -62,3 +65,92 @@ INSTANTIATE_TEST_SUITE_P( ::testing::Combine(::testing::Values("mkdwarfs", "dwarfs", "dwarfsck", "dwarfsextract"), ::testing::Bool())); + +TEST(pager_test, find_pager_program) { + auto resolver = [](std::filesystem::path const& name) { + std::map 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({"-R"}), pager->args); + } + + { + auto pager = find_pager_program(os); + ASSERT_TRUE(pager); + EXPECT_EQ("/whatever/bin/less", pager->name); + EXPECT_EQ(std::vector({"-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_TRUE(pager->args.empty()); + } + + 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()); + } + } +} diff --git a/test/test_helpers.cpp b/test/test_helpers.cpp index 994dfaa1..01bc572b 100644 --- a/test/test_helpers.cpp +++ b/test/test_helpers.cpp @@ -516,20 +516,25 @@ os_access_mock::thread_get_cpu_time(std::thread::id tid, return real_os_->thread_get_cpu_time(tid, ec); } -std::optional find_binary(std::string_view name) { - auto path_str = std::getenv("PATH"); - if (!path_str) { - return std::nullopt; +std::filesystem::path +os_access_mock::find_executable(std::filesystem::path const& name) const { + if (executable_resolver_) { + return executable_resolver_(name); } - std::vector path; - folly::split(':', path_str, path); + return real_os_->find_executable(name); +} - for (auto dir : path) { - auto cand = fs::path(dir) / name; - if (fs::exists(cand) and ::access(cand.string().c_str(), X_OK) == 0) { - return cand; - } +void os_access_mock::set_executable_resolver( + executable_resolver_type resolver) { + executable_resolver_ = std::move(resolver); +} + +std::optional find_binary(std::string_view name) { + os_access_generic os; + + if (auto path = os.find_executable(name); !path.empty()) { + return path; } return std::nullopt; diff --git a/test/test_helpers.h b/test/test_helpers.h index 3b7867aa..2cc41efb 100644 --- a/test/test_helpers.h +++ b/test/test_helpers.h @@ -73,6 +73,9 @@ class os_access_mock : public os_access { std::variant, std::unique_ptr>; + using executable_resolver_type = + std::function; + os_access_mock(); ~os_access_mock(); @@ -126,6 +129,11 @@ class os_access_mock : public os_access { std::chrono::nanoseconds thread_get_cpu_time(std::thread::id tid, std::error_code& ec) const override; + std::filesystem::path + find_executable(std::filesystem::path const& name) const override; + + void set_executable_resolver(executable_resolver_type resolver); + std::set get_failed_paths() const; std::vector< @@ -150,6 +158,7 @@ class os_access_mock : public os_access { std::map map_file_errors_; std::map env_; std::shared_ptr real_os_; + executable_resolver_type executable_resolver_; }; class script_mock : public script {