taskolib / libgit4cpp

C++ wrapper for libgit2 with limited functionality
https://taskolib.github.io/libgit4cpp/
GNU Lesser General Public License v2.1
1 stars 0 forks source link

Remove debug output - minimal version #14

Closed Finii closed 6 months ago

Finii commented 6 months ago

This is in some ways less and more than

It does

Apart from that, and ahead of #13 it renames LibGitPointer to LibGitObject and later does not directly use that anymore but uses 'named types' (aliases) for the various objects.

I believe Pointer is a misnomer. Of course technically it just holds a pointer, and it just calls 'free()' in the appropriate moment. But that is exactly the definiting of 'owing' the structure. So from a C++ point of view this 'wrapper object' is the object.

This is why I wanted to have (for example) a LibGitCommit which looks like a C++ object and behaves like a C++ object albeit under the hood it is a c pointer. Well, it is, but that does not need to show by the name.

Which does look more like 'libgit2 for cpp'? :

// implemented in this PR
LibGitCommit GitRepository::get_commit(unsigned int count);

 // previous naming
LibGitPointer<git_commit> GitRepository::get_commit(unsigned int count);

Fixes: #5 Fixes: #15

Finii commented 6 months ago

I'm not sure about the last commit

629cea2a GitRepository: Make more members const

I would do that, though, but if you are uncomfortable with const_cast we can drop that commit.

And when writing this, I can hardly follow why people want const west... For me the east const keeps the two things together we also use when speaking. "That object is handed over by const-ref", then write that?

// how I finally wrote it in the PR
RepoState collect_status(const LibGitStatusList& status) const;

// what I instrinctically write first and needed to 'correct'
RepoState collect_status(LibGitStatusList const& status) const;

Just trying to hand out east-const-wristbands

signal-2023-12-11-091402

Finii commented 6 months ago

Please review again.

You said / noted that LibGitPointer was inteded as a std::unique_ptr equivalent.

Then, why a special class AT ALL? Just use unique_ptr if you want one?

Drops a lot of hand written code, maybe this is what is really wanted?

Will revert or keep, whatever you think is intended.

Finii commented 6 months ago

Don't get me wrong, I like the aliases. Still, LibGitPtr is just a unique_ptr to me, and therefore I find that name more logical than LibGitObject. But as I wrote, I can live with both.

Ah yes. Its a unique_ptr for you -> then use a unique_ptr πŸ˜„

But maybe the header file needs to be renamed. Any ideas?

alt-graph commented 6 months ago

Don't get me wrong, I like the aliases. Still, LibGitPtr is just a unique_ptr to me, and therefore I find that name more logical than LibGitObject. But as I wrote, I can live with both.

Ah yes. Its a unique_ptr for you -> then use a unique_ptr πŸ˜„

Nice! I did not know that could be done.

But maybe the header file needs to be renamed. Any ideas?

Something vague like "aliases.h" or "types.h" maybe?

Finii commented 6 months ago
Finii commented 6 months ago

Now doing step 3, hold on

Finii commented 6 months ago

Squashed a lot of commits but keeping some. I guess this is about the gist of what this does.

Ready to merge from my side β›±