fix memry issue and add test

QuantStack · JohanMabille · Aug 28, 2025 · Aug 26, 2025 · Aug 26, 2025 · Aug 27, 2025
commit b08b8caacf0a2dea00ffa1c0d8e4c2ec328b9e54
diff --git a/src/subcommand/log_subcommand.cpp b/src/subcommand/log_subcommand.cpp
@@ -1,19 +1,12 @@
-// #include <iostream>
-// #include <ostream>
-// #include <string>
-
+#include <format>
 #include <git2.h>
 #include <git2/revwalk.h>
 #include <git2/types.h>
+#include <string_view>
 
 #include "log_subcommand.hpp"
 #include "../wrapper/repository_wrapper.hpp"
-
-// TODO: put in another file
-/** Size (in bytes) of a raw/binary sha1 oid */
-#define GIT_OID_SHA1_SIZE       20
-/** Size (in bytes) of a hex formatted sha1 oid */
-#define GIT_OID_SHA1_HEXSIZE   (GIT_OID_SHA1_SIZE * 2)
+#include "../wrapper/commit_wrapper.hpp"
 
 log_subcommand::log_subcommand(const libgit2_object&, CLI::App& app)
 {
@@ -26,7 +19,7 @@ log_subcommand::log_subcommand(const libgit2_object&, CLI::App& app)
     sub->callback([this]() { this->run(); });
 };
 
-void print_time(git_time intime, const char *prefix)
+void print_time(git_time intime, std::string prefix)
 {
 	char sign, out[32];
 	struct tm *intm;
@@ -37,7 +30,9 @@ void print_time(git_time intime, const char *prefix)
 	if (offset < 0) {
 		sign = '-';
 		offset = -offset;
-	} else {
+	}
+	else
+	{
 		sign = '+';
 	}
 
@@ -49,38 +44,37 @@ void print_time(git_time intime, const char *prefix)
 	intm = gmtime(&t);
 	strftime(out, sizeof(out), "%a %b %e %T %Y", intm);
 
-	printf("%s%s %c%02d%02d\n", prefix, out, sign, hours, minutes);
+	std::cout << prefix << out << " " << sign << std::format("{:02d}", hours) << std::format("{:02d}", minutes) <<std::endl;
 }
 
 void print_commit(const commit_wrapper& commit, std::string m_format_flag)
 {
-    // TODO: put in commit_wrapper ?
-    char buf[GIT_OID_SHA1_HEXSIZE + 1];
-    int i, count;
-
-    git_oid_tostr(buf, sizeof(buf), &commit.oid());
-    // TODO end
+    std::string buf = commit.commit_oid_tostr();
 
     signature_wrapper author = signature_wrapper::get_commit_author(commit);
     signature_wrapper committer = signature_wrapper::get_commit_committer(commit);
 
     std::cout << "\033[0;33m" << "commit " << buf << "\033[0m" << std::endl;
-    std::cout << "Author:\t" <<  author.name() << " " << author.email() << std::endl;
-    if (m_format_flag=="full")
-    {
-        std::cout << "Commit:\t" <<  committer.name() << " " << committer.email() << std::endl;
-    }
-    else if (m_format_flag=="fuller")
+    if (m_format_flag=="fuller")
     {
-        print_time(author.when(), "AuthorDate:\t");
-        std::cout << "Commit:\t" <<  committer.name() << " " << committer.email() << std::endl;
-        print_time(committer.when(), "CommitDate:\t");
+        std::cout << "Author:\t    " <<  author.name() << " " << author.email() << std::endl;
+        print_time(author.when(), "AuthorDate: ");
+        std::cout << "Commit:\t    " <<  committer.name() << " " << committer.email() << std::endl;
+        print_time(committer.when(), "CommitDate: ");
     }
     else
     {
-        print_time(author.when(), "Date:\t");
+        std::cout << "Author:\t" <<  author.name() << " " << author.email() << std::endl;
+        if (m_format_flag=="full")
+        {
+            std::cout << "Commit:\t" <<  committer.name() << " " << committer.email() << std::endl;
+        }
+        else
+        {
+            print_time(author.when(), "Date:\t");
+        }
     }
-    std::cout << git_commit_message(commit) << "\n" << std::endl;
+    std::cout << "\n    " << git_commit_message(commit) << "\n" << std::endl;
 }
 
 void log_subcommand::run()

diff --git a/src/wrapper/commit_wrapper.cpp b/src/wrapper/commit_wrapper.cpp
@@ -20,3 +20,9 @@ const git_oid& commit_wrapper::oid() const
 {
     return *git_commit_id(p_resource);
 }
+
+std::string commit_wrapper::commit_oid_tostr() const
+{
+    char buf[GIT_OID_SHA1_HEXSIZE + 1];
+    return git_oid_tostr(buf, sizeof(buf), &this->oid());
+}
diff --git a/src/wrapper/commit_wrapper.hpp b/src/wrapper/commit_wrapper.hpp
@@ -19,6 +19,7 @@ class commit_wrapper : public wrapper_base<git_commit>
     operator git_object*() const noexcept;
 
     const git_oid& oid() const;
+    std::string commit_oid_tostr() const;
 
 private:
 

diff --git a/src/wrapper/signature_wrapper.cpp b/src/wrapper/signature_wrapper.cpp
@@ -4,7 +4,10 @@
 
 signature_wrapper::~signature_wrapper()
 {
-    git_signature_free(p_resource);
+    if (m_ownership)
+    {
+        git_signature_free(p_resource);
+    }
     p_resource=nullptr;
 }
 
@@ -35,12 +38,14 @@ signature_wrapper signature_wrapper::get_commit_author(const commit_wrapper& cw)
 {
     signature_wrapper author;
     author.p_resource = const_cast<git_signature*>(git_commit_author(cw));
+    author.m_ownership = false;
     return author;
 }
 
 signature_wrapper signature_wrapper::get_commit_committer(const commit_wrapper& cw)
 {
-    signature_wrapper author;
-    author.p_resource = const_cast<git_signature*>(git_commit_committer(cw));
-    return author;
+    signature_wrapper committer;
+    committer.p_resource = const_cast<git_signature*>(git_commit_committer(cw));
+    committer.m_ownership = false;
+    return committer;
 }
diff --git a/src/wrapper/signature_wrapper.hpp b/src/wrapper/signature_wrapper.hpp
@@ -31,4 +31,5 @@ class signature_wrapper : public wrapper_base<git_signature>
 private:
 
     signature_wrapper() = default;
+    bool m_ownership=true;
 };
diff --git a/test/test_log.py b/test/test_log.py
@@ -0,0 +1,36 @@
+import subprocess
+
+import pytest
+
+@pytest.mark.parametrize("format_flag", ["", "--format=full", "--format=fuller"])
+def test_log(xtl_clone, git_config, git2cpp_path, tmp_path, monkeypatch, format_flag):
+    assert (tmp_path / "xtl").exists()
+    xtl_path = tmp_path / "xtl"
+
+    p = xtl_path / "mook_file.txt"
+    p.write_text('')
+
+    cmd_add = [git2cpp_path, 'add', "mook_file.txt"]
+    p_add = subprocess.run(cmd_add, cwd=xtl_path, text=True)
+    assert p_add.returncode == 0
+
+    cmd_commit = [git2cpp_path, 'commit', "-m", "test commit"]
+    p_commit = subprocess.run(cmd_commit, cwd=xtl_path, text=True)
+    assert p_commit.returncode == 0
+
+    cmd_log = [git2cpp_path, 'log']
+    if format_flag != "":
+        cmd_log.append(format_flag)
+    p_log = subprocess.run(cmd_log, capture_output=True, cwd=xtl_path, text=True)
+    assert p_log.returncode == 0
+    assert "Jane Doe" in p_log.stdout
+    assert "test commit" in p_log.stdout
+
+    if format_flag == "":
+        assert "Commit" not in p_log.stdout
+    else:
+        assert "Commit" in p_log.stdout
+        if format_flag == "--format=full":
+            assert "Date" not in p_log.stdout
+        else:
+            assert "CommitDate" in p_log.stdout