address review cooment

Author: SandrineP

src/subcommand/log_subcommand.cpp

@@ -16,7 +16,7 @@ log_subcommand::log_subcommand(const libgit2_object&, CLI::App& app)
 {
     auto *sub = app.add_subcommand("log", "Shows commit logs");
 
-    sub->add_option("--format", m_format_flag, "Pretty-print the contents of the commit logs in a given format, where <format> can be one of full and fuller");
+    sub->add_option("--format", m_format_flag, "Pretty-print the contents of the commit logs in a given format, where <format> can be one of full, fuller or oneline");
     sub->add_option("-n,--max-count", m_max_count_flag, "Limit the output to <number> commits.");
     sub->add_flag("--abbrev-commit", m_abbrev_commit_flag, "Instead of showing the full 40-byte hexadecimal commit object name, show a prefix that names the object uniquely. --abbrev=<n> (which also modifies diff output, if it is displayed) option can be used to specify the minimum length of the prefix.");
     sub->add_option("--abbrev", m_abbrev, "Instead of showing the full 40-byte hexadecimal object name in diff-raw format output and diff-tree header lines, show the shortest prefix that is at least <n> hexdigits long that uniquely refers the object.");
@@ -230,7 +230,9 @@ void log_subcommand::print_commit(repository_wrapper& repo, const commit_wrapper
 
     std::string message = commit.message();
     while (!message.empty() && message.back() == '\n')
+    {
         message.pop_back();
+    }
 
     if (oneline)
     {

src/subcommand/revparse_subcommand.cpp

@@ -8,10 +8,9 @@ revparse_subcommand::revparse_subcommand(const libgit2_object&, CLI::App& app)
 
     auto* bare_opt = sub->add_flag("--is-bare-repository", m_is_bare_repository_flag, "When the repository is bare print \"true\", otherwise \"false\".");
     auto* shallow_opt = sub->add_flag("--is-shallow-repository", m_is_shallow_repository_flag, "When the repository is shallow print \"true\", otherwise \"false\".");
+    auto* rev_opt = sub->add_option("<rev>", m_revisions, "Revision(s) to parse (e.g. HEAD, main, HEAD~1, dae86e, ...)");
 
-    sub->add_option("<rev>", m_revisions, "Revision(s) to parse (e.g. HEAD, main, HEAD~1, dae86e, ...)");
-
-    sub->parse_complete_callback([this, sub, bare_opt, shallow_opt]() {
+    sub->parse_complete_callback([this, sub, bare_opt, shallow_opt, rev_opt]() {
         for (CLI::Option* opt : sub->parse_order())
         {
             if (opt == bare_opt)
@@ -22,6 +21,10 @@ revparse_subcommand::revparse_subcommand(const libgit2_object&, CLI::App& app)
             {
                 m_queries_in_order.push_back("is_shallow");
             }
+            else if (opt == rev_opt)
+            {
+                m_queries_in_order.push_back("is_rev");
+            }
         }
     });
 
@@ -33,6 +36,7 @@ void revparse_subcommand::run()
     auto directory = get_current_git_path();
     auto repo = repository_wrapper::open(directory);
 
+    size_t i = 0;
     if (!m_queries_in_order.empty())
     {
         for (const auto& q : m_queries_in_order)
@@ -41,31 +45,25 @@ void revparse_subcommand::run()
             {
                 std::cout << std::boolalpha << repo.is_bare() << std::endl;
             }
-            if (q == "is_shallow")
+            else if (q == "is_shallow")
             {
                 std::cout << std::boolalpha << repo.is_shallow() << std::endl;
             }
-        }
-        return;
-    }
+            else if (q == "is_rev")
+            {
+                const auto& rev = m_revisions[i];
+                auto obj = repo.revparse_single(rev.c_str());
 
-    if (!m_revisions.empty())
-    {
-        for (const auto& rev : m_revisions)
-        {
-            auto obj = repo.revparse_single(rev.c_str());
+                if (!obj.has_value())
+                {
+                    throw git_exception("bad revision '" + rev + "'", git2cpp_error_code::BAD_ARGUMENT);
+                }
 
-            if (!obj.has_value())
-            {
-                throw git_exception("bad revision '" + rev + "'", git2cpp_error_code::BAD_ARGUMENT);
-                return;
+                auto oid = obj.value().oid();
+                std::cout << git_oid_tostr_s(&oid) << std::endl;
+                i += 1;
             }
-
-            auto oid = obj.value().oid();
-            std::cout << git_oid_tostr_s(&oid) << std::endl;
         }
-        return;
     }
-
-    std::cout << "revparse only supports --is-bare-repository, --is-shallow-repository and parsing revisions for now" << std::endl;
+    return;
 }

test/test_revparse.py

@@ -70,6 +70,55 @@ def test_revparse_multiple_revs(repo_init_with_commit, git2cpp_path, tmp_path):
     assert p.returncode == 0
 
     lines = p.stdout.splitlines()
+    print()
     assert len(lines) == 2
     assert all(len(x) == 40 for x in lines)
     assert lines[0] != lines[1]
+
+
+def test_revparse_multiple_opts(git2cpp_path, tmp_path, run_in_tmp_path):
+    """Test the options are printed in order"""
+    url = "https://github.com/xtensor-stack/xtl.git"
+    cmd = [git2cpp_path, "clone", "--depth", "2", url]
+    p = subprocess.run(cmd, capture_output=True, text=True, cwd=tmp_path)
+    assert p.returncode == 0
+    assert (tmp_path / "xtl").exists()
+
+    xtl_path = tmp_path / "xtl"
+
+    p = subprocess.run(
+        [
+            git2cpp_path,
+            "rev-parse",
+            "HEAD",
+            "--is-shallow-repository",
+            "--is-bare-repository",
+            "HEAD~1",
+        ],
+        capture_output=True,
+        text=True,
+        cwd=xtl_path,
+    )
+    assert p.returncode == 0
+
+    lines = p.stdout.splitlines()
+    assert len(lines) == 4
+    assert len(lines[0]) == 40
+    assert len(lines[3]) == 40
+    assert lines[0] != lines[1]
+    assert "true" in lines[1]
+    assert "false" in lines[2]
+
+
+def test_revparse_errors(repo_init_with_commit, git2cpp_path, tmp_path):
+    assert (tmp_path / "initial.txt").exists()
+
+    rev_cmd = [git2cpp_path, "rev-parse", "HEAD~1"]
+    p_rev = subprocess.run(rev_cmd, capture_output=True, text=True, cwd=tmp_path)
+    assert p_rev.returncode == 129
+    assert "bad revision" in p_rev.stderr
+
+    opt_cmd = [git2cpp_path, "rev-parse", "--parseopt"]
+    p_opt = subprocess.run(opt_cmd, capture_output=True, text=True, cwd=tmp_path)
+    assert p_opt.returncode != 0
+    assert "The following argument was not expected:" in p_opt.stderr

test/test_tag.py

@@ -135,8 +135,8 @@ def test_tag_delete_nonexistent(repo_init_with_commit, git2cpp_path, tmp_path):
     assert "not found" in p_delete.stderr
 
 
-@pytest.mark.parametrize("list_flag", ["-l", "--list"])
-def test_tag_list_with_flag(
+@pytest.mark.parametrize("list_flag", ["", "-l", "--list"])
+def test_tag_list(
     repo_init_with_commit, commit_env_config, git2cpp_path, tmp_path, list_flag
 ):
     """Test listing tags with -l or --list flag."""
@@ -350,3 +350,18 @@ def test_tag_annotate_flag_requires_message(
     p = subprocess.run(create_cmd, capture_output=True, cwd=tmp_path, text=True)
     assert p.returncode != 0
     assert "requires -m" in p.stderr
+
+
+def test_tag_errors(repo_init_with_commit, commit_env_config, git2cpp_path, tmp_path):
+    assert (tmp_path / "initial.txt").exists()
+
+    # Test that -a/--annotate without -m fails.
+    create_cmd = [git2cpp_path, "tag", "-a", "v1.0.0"]
+    p_create = subprocess.run(create_cmd, capture_output=True, cwd=tmp_path, text=True)
+    assert p_create.returncode != 0
+    assert "requires -m" in p_create.stderr
+
+    # Test that command fails when no message
+    del_cmd = [git2cpp_path, "tag", "-d"]
+    p_del = subprocess.run(del_cmd, capture_output=True, cwd=tmp_path, text=True)
+    assert p_del.returncode != 0