Add tracking info for log subcommand by SandrineP · Pull Request #106 · QuantStack/git2cpp

SandrineP · 2026-02-25T11:23:21Z

Fix #90
Add tracking info for log subcommand

codecov · 2026-02-25T15:57:42Z

Codecov Report

❌ Patch coverage is 98.90110% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.83%. Comparing base (c460fce) to head (8e58bbc).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/subcommand/log_subcommand.cpp	98.90%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #106      +/-   ##
==========================================
+ Coverage   86.18%   86.83%   +0.65%     
==========================================
  Files          60       60              
  Lines        2186     2340     +154     
  Branches      244      276      +32     
==========================================
+ Hits         1884     2032     +148     
- Misses        302      308       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ianthomas23 · 2026-02-26T10:44:12Z

The tag and branch information looks really good, and the whitespace between commits is different but not the same as in real git:

SandrineP · 2026-02-26T11:17:00Z

The tag and branch information looks really good, and the whitespace between commits is different but not the same as in real git:

I looked at the extra newlines, and I thought that there was always a trailing one at the end of the message in git2cpp. So I changed the code a bit and in the tests I made, there was only a newline at the very end that is not there with git. The rest was identical. So I thought it was fine, but apparently not ! I'll have another look.

JohanMabille · 2026-02-26T16:44:00Z

src/subcommand/log_subcommand.cpp

+        }
+    }
+
+    git_strarray_dispose(&tag_names);


This is not exception-safe. However, we cannot use the git_strarray_wrapper here, since this class actually wraps a git_strarray whose elements point to already allocating strings (and thus, git_strarray_dispose is not called in the destructor to avoid double deletion).

Which leads me to think that the current git_strarray_wrapper is probably badly named. And that we should have a git_strarray_wrapper that frees both git_strarray elements and the array itself.

But this implies refactoring and goes beyond the scope of this PR, so let's add a TODO here and open an issue to track it when we merge this PR.

JohanMabille · 2026-02-26T16:45:13Z

src/subcommand/log_subcommand.cpp

+            std::string branch_name;
+            branch_name = branch->name();


Suggested change

std::string branch_name;

branch_name = branch->name();

std::string branch_name = branch->name();

I wrote it this way because it's not happy with the one line option:
error: conversion from 'std::string_view' {aka 'std::basic_string_view<char>'} to non-scalar type 'std::string' {aka 'std::__cxx11::basic_string<char>'} requested

How about

std::string branch_name(branch->name());

instead, which works for me on macos?

I missed that the return type of branch::name was std::string_view. In that case the correct solution is the line from Ian.

JohanMabille · 2026-02-26T16:47:29Z

src/subcommand/log_subcommand.cpp

+    return tags;
+}
+
+std::vector<std::string> get_branches_for_commit(repository_wrapper& repo, git_branch_t type, const git_oid* commit_oid, const std::string exclude_branch)


commit_oid should be passed by reference insteadof pointer.

JohanMabille · 2026-02-26T16:47:56Z

src/subcommand/log_subcommand.cpp

+    }
+};
+
+commit_refs get_refs_for_commit(repository_wrapper& repo, const git_oid* commit_oid)


commit_oid should be passed by reference instead of pointer.

JohanMabille · 2026-02-26T16:48:52Z

src/subcommand/log_subcommand.cpp

+    return refs;
+}
+
+void print_refs(commit_refs refs)


refs should be passed by constant reference.

ianthomas23

The whitespace looks excellent now thanks, and I've checked all of Johan's comments too and this is ready to merge.

I've left the one comment about the branch_name string, feel free to make that change or not.

JohanMabille · 2026-02-27T14:30:22Z

src/subcommand/log_subcommand.cpp

 }

-void print_commit(const commit_wrapper& commit, std::string m_format_flag)
+std::vector<std::string> get_tags_for_commit(repository_wrapper& repo, const git_oid* commit_oid)


The commit_oid parameter should be passed by reference.

ianthomas23 · 2026-02-27T15:08:06Z

I've confirmed that the extra 2 proposed changes have been implemented, so I am merging.

SandrineP force-pushed the log_tracking branch from cb306a0 to 05a30ec Compare February 25, 2026 12:18

Add tracking info

81992bd

SandrineP force-pushed the log_tracking branch from 05a30ec to 81992bd Compare February 25, 2026 15:53

SandrineP marked this pull request as ready for review February 25, 2026 15:54

SandrineP changed the title ~~Add tracking info~~ Add tracking info for log subcommand Feb 26, 2026

JohanMabille requested changes Feb 26, 2026

View reviewed changes

SandrineP mentioned this pull request Feb 27, 2026

Refactor git_strarray_wrapper #108

Open

address review comments

807cb09

SandrineP force-pushed the log_tracking branch from cb729f2 to 807cb09 Compare February 27, 2026 09:38

SandrineP added the maintenance label Feb 27, 2026

ianthomas23 approved these changes Feb 27, 2026

View reviewed changes

JohanMabille reviewed Feb 27, 2026

View reviewed changes

address review comments

8e58bbc

ianthomas23 merged commit 1ba9701 into QuantStack:main Feb 27, 2026
7 of 8 checks passed

SandrineP deleted the log_tracking branch February 27, 2026 15:39

	std::string branch_name;
	branch_name = branch->name();
	std::string branch_name = branch->name();

Conversation

SandrineP commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ianthomas23 commented Feb 26, 2026

Uh oh!

SandrineP commented Feb 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianthomas23 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ianthomas23 commented Feb 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SandrineP commented Feb 25, 2026 •

edited

Loading

codecov bot commented Feb 25, 2026 •

edited

Loading

ianthomas23 left a comment •

edited

Loading