address review comments

Author: SandrineP

src/subcommand/checkout_subcommand.cpp

@@ -56,26 +56,6 @@ void checkout_subcommand::run()
     }
 }
 
-std::optional<annotated_commit_wrapper> checkout_subcommand::resolve_local_ref
-(
-    const repository_wrapper& repo,
-    const std::string& target_name
-)
-{
-    if (auto ref = repo.find_reference_dwim(target_name))
-    {
-        return repo.find_annotated_commit(*ref);
-    }
-    else if (auto obj = repo.revparse_single(target_name))
-    {
-        return repo.find_annotated_commit(obj->oid());
-    }
-    else
-    {
-        return std::nullopt;
-    }
-}
-
 annotated_commit_wrapper checkout_subcommand::create_local_branch
 (
     repository_wrapper& repo,

src/subcommand/checkout_subcommand.hpp

@@ -17,12 +17,6 @@ class checkout_subcommand
 
 private:
 
-    std::optional<annotated_commit_wrapper> resolve_local_ref
-    (
-        const repository_wrapper& repo,
-        const std::string& target_name
-    );
-
     annotated_commit_wrapper create_local_branch
     (
         repository_wrapper& repo,

src/subcommand/merge_subcommand.cpp

@@ -2,7 +2,7 @@
 #include <git2/types.h>
 
 #include "merge_subcommand.hpp"
-#include "../wrapper/repository_wrapper.hpp"
+// #include "../wrapper/repository_wrapper.hpp"
 
 
 merge_subcommand::merge_subcommand(const libgit2_object&, CLI::App& app)
@@ -14,7 +14,7 @@ merge_subcommand::merge_subcommand(const libgit2_object&, CLI::App& app)
     sub->callback([this]() { this->run(); });
 }
 
-annotated_commit_list_wrapper resolve_heads(const repository_wrapper& repo, std::vector<std::string> m_branches_to_merge)
+annotated_commit_list_wrapper merge_subcommand::resolve_heads(const repository_wrapper& repo)
 {
     std::vector<annotated_commit_wrapper> commits_to_merge;
     commits_to_merge.reserve(m_branches_to_merge.size());
@@ -30,7 +30,7 @@ annotated_commit_list_wrapper resolve_heads(const repository_wrapper& repo, std:
     return annotated_commit_list_wrapper(std::move(commits_to_merge));
 }
 
-void perform_fastforward(repository_wrapper& repo, const git_oid* target_oid, int is_unborn)
+void perform_fastforward(repository_wrapper& repo, const git_oid target_oid, int is_unborn)
 {
     const git_checkout_options ff_checkout_options = GIT_CHECKOUT_OPTIONS_INIT;
 
@@ -45,13 +45,13 @@ void perform_fastforward(repository_wrapper& repo, const git_oid* target_oid, in
             return repo->find_reference("HEAD");
         }
     };
-    auto target_ref = lambda_get_target_ref(&repo, is_unborn);
+    reference_wrapper target_ref = lambda_get_target_ref(&repo, is_unborn);
 
-    auto target = repo.find_object(target_oid, GIT_OBJECT_COMMIT);
+    object_wrapper target = repo.find_object(target_oid, GIT_OBJECT_COMMIT);
 
-    repo.checkout_tree(target, &ff_checkout_options);
+    repo.checkout_tree(target, ff_checkout_options);
 
-    auto new_target_ref = target_ref.new_ref(target_oid);
+    target_ref.write_new_ref(target_oid);
 }
 
 void merge_subcommand::run()
@@ -68,7 +68,7 @@ void merge_subcommand::run()
 
     git_merge_analysis_t analysis;
 	git_merge_preference_t preference;
-	annotated_commit_list_wrapper commits_to_merge = resolve_heads(repo, m_branches_to_merge);
+	annotated_commit_list_wrapper commits_to_merge = resolve_heads(repo);
 	size_t num_commits_to_merge = commits_to_merge.size();
 	git_annotated_commit** c_commits_to_merge = commits_to_merge;
 	auto commits_to_merge_const = const_cast<const git_annotated_commit**>(c_commits_to_merge);
@@ -83,7 +83,6 @@ void merge_subcommand::run()
              (analysis & GIT_MERGE_ANALYSIS_FASTFORWARD &&
              !(preference & GIT_MERGE_PREFERENCE_NO_FASTFORWARD)))
     {
-        const git_oid* target_oid;
         if (analysis & GIT_MERGE_ANALYSIS_UNBORN)
         {
             std::cout << "Unborn" << std::endl;
@@ -93,7 +92,8 @@ void merge_subcommand::run()
             std::cout << "Fast-forward" << std::endl;
         }
         const annotated_commit_wrapper& commit = commits_to_merge.front();
-        target_oid = &commit.oid();
+        const git_oid target_oid = commit.oid();
+        // Since this is a fast-forward, there can be only one merge head.
         assert(num_commits_to_merge == 1);
         perform_fastforward(repo, target_oid, (analysis & GIT_MERGE_ANALYSIS_UNBORN));
     }

src/subcommand/merge_subcommand.hpp

@@ -3,6 +3,7 @@
 #include <CLI/CLI.hpp>
 
 #include "../utils/common.hpp"
+#include "../wrapper/repository_wrapper.hpp"
 
 class merge_subcommand
 {
@@ -12,5 +13,8 @@ class merge_subcommand
     void run();
 
 private:
+
+    annotated_commit_list_wrapper resolve_heads(const repository_wrapper& repo);
+
     std::vector<std::string> m_branches_to_merge;
 };

src/wrapper/annotated_commit_wrapper.cpp

@@ -1,5 +1,4 @@
 #include "../wrapper/annotated_commit_wrapper.hpp"
-#include <iostream>
 
 annotated_commit_wrapper::annotated_commit_wrapper(git_annotated_commit* commit)
     : base_type(commit)
@@ -22,38 +21,3 @@ std::string_view annotated_commit_wrapper::reference_name() const
     const char* res = git_annotated_commit_ref(*this);
     return res ? res : std::string_view{};
 }
-
-annotated_commit_list_wrapper::annotated_commit_list_wrapper(std::vector<annotated_commit_wrapper> annotated_commit_list)
-    : m_annotated_commit_list(std::move(annotated_commit_list))
-{
-    p_resource = new git_annotated_commit*[m_annotated_commit_list.size()];
-    for (size_t i=0; i<m_annotated_commit_list.size(); ++i)
-    {
-        p_resource[i] = m_annotated_commit_list[i];
-    }
-}
-
-annotated_commit_list_wrapper::~annotated_commit_list_wrapper()
-{
-    delete[] p_resource;
-    p_resource = nullptr;
-}
-
-size_t annotated_commit_list_wrapper::size() const
-{
-    return m_annotated_commit_list.size();
-}
-
-annotated_commit_wrapper annotated_commit_list_wrapper::front()
-{
-    return annotated_commit_wrapper(std::move(m_annotated_commit_list.front()));
-}
-
-void annotated_commit_list_wrapper::print() const
-{
-    std::cout << "aclw - p_resource = " << p_resource <<std::endl;
-    for (size_t i=0; i<m_annotated_commit_list.size(); ++i)
-    {
-        std::cout << "aclw - p_resource[i] = :" << p_resource[i] << std::endl;
-    }
-}

src/wrapper/annotated_commit_wrapper.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
 #include <string_view>
-#include <vector>
 
 #include <git2.h>
 
@@ -28,25 +27,4 @@ class annotated_commit_wrapper : public wrapper_base<git_annotated_commit>
     friend class repository_wrapper;
 };
 
-class annotated_commit_list_wrapper : public wrapper_base<git_annotated_commit*>
-{
-public:
-
-    using base_type = wrapper_base<git_annotated_commit*>;
-
-    explicit annotated_commit_list_wrapper(std::vector<annotated_commit_wrapper> annotated_commit_list);
-
-    ~annotated_commit_list_wrapper();
-
-    annotated_commit_list_wrapper(annotated_commit_list_wrapper&&) noexcept = default;
-    annotated_commit_list_wrapper& operator=(annotated_commit_list_wrapper&&) noexcept = default;
-
-    size_t size() const;
-    annotated_commit_wrapper front();
-    void print() const;
-
-private:
-
-    std::vector<annotated_commit_wrapper> m_annotated_commit_list;
-
-};
+using annotated_commit_list_wrapper = list_wrapper<annotated_commit_wrapper>;

src/wrapper/commit_wrapper.cpp

@@ -26,23 +26,3 @@ std::string commit_wrapper::commit_oid_tostr() const
     char buf[GIT_OID_SHA1_HEXSIZE + 1];
     return git_oid_tostr(buf, sizeof(buf), &this->oid());
 }
-
-commit_list_wrapper::commit_list_wrapper(std::vector<commit_wrapper> commit_list)
-{
-    git_commit** p_resource = new git_commit*[m_commit_list.size()];
-    for (size_t i=0; i<m_commit_list.size(); ++i)
-    {
-        p_resource[i] = m_commit_list[i];
-    }
-}
-
-commit_list_wrapper::~commit_list_wrapper()
-{
-    delete[] p_resource;
-    p_resource = nullptr;
-}
-
-size_t commit_list_wrapper::size() const
-{
-    return m_commit_list.size();
-}

src/wrapper/commit_wrapper.hpp

@@ -30,23 +30,4 @@ class commit_wrapper : public wrapper_base<git_commit>
     friend class reference_wrapper;
 };
 
-class commit_list_wrapper : public wrapper_base<git_commit*>
-{
-public:
-
-    using base_type = wrapper_base<git_commit*>;
-
-    explicit commit_list_wrapper(std::vector<commit_wrapper> commit_list);
-
-    ~commit_list_wrapper();
-
-    commit_list_wrapper(commit_list_wrapper&&) noexcept = default;
-    commit_list_wrapper& operator=(commit_list_wrapper&&) noexcept = default;
-
-    size_t size() const;
-
-private:
-
-    std::vector<commit_wrapper> m_commit_list;
-
-};
+using commit_list_wrapper = list_wrapper<commit_wrapper>;

src/wrapper/refs_wrapper.cpp

@@ -30,9 +30,9 @@ const git_oid* reference_wrapper::target() const
     return git_reference_target(p_resource);
 }
 
-reference_wrapper reference_wrapper::new_ref(const git_oid* target_oid)
+reference_wrapper reference_wrapper::write_new_ref(const git_oid target_oid)
 {
     git_reference* new_ref;
-    throw_if_error(git_reference_set_target(&new_ref, p_resource, target_oid, NULL));
+    throw_if_error(git_reference_set_target(&new_ref, p_resource, &target_oid, NULL));
     return reference_wrapper(new_ref);
 }

src/wrapper/refs_wrapper.hpp

@@ -23,7 +23,7 @@ class reference_wrapper : public wrapper_base<git_reference>
     std::string short_name() const;
     bool is_remote() const;
     const git_oid* target() const;
-    reference_wrapper new_ref(const git_oid* target_oid);
+    reference_wrapper write_new_ref(const git_oid target_oid);
 
     template <class W>
     W peel() const;