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

Allow globbing in add() and update() AND fix defunct reset(0) #7

Closed Finii closed 7 months ago

Finii commented 7 months ago

[why]

We can only add specific files are all files. It is not possible to add a whole subdirectory via glob.

With add() we can not add removed files. For some unknown reason add() is public but update() is not.

[how]

Forward glob string to libgit2 add() function.

Make update() public. Allow globs to be specified for update(), to allow updating whole subdirectories and not only the complete repository.

Finii commented 7 months ago

I have a refactor PR for this pending, and additionally I now have problems using git reset... sigh. Rewrote the other lib's commit stuff. Oh my!

Ah, and I want dump-to-stream ops, that comes next. debugging blindly or writing always anew an output function :roll_eyes:

Finii commented 7 months ago

additionally I now have problems using git reset

test_GitRepository.cc:     * TODO: Fuktionalitaet von git reset implementieren

:sob:

Finii commented 7 months ago

Hmm, repaired reset().

Which is unrelated to the initial topic of this PR.

I would really just like to push in the corrections, I can work on the documentation and tests later. But at the moment I struggle with taskolib and we need the fixes to troddle along there.

I would propose: If these changes are good enough, just doc and tests missing, we could pull them already and I work on the docs tomorrow or something. Well, after taskolib that is. But I guess that is almost finished now.

What do you think?

And as I said before, I have lots of commits for this repo flying around anyhow, that could be a nice next PR here.

Edit:

I also find the referencing with numbers (example 4) as HEAD~4 very biased. What is with HEAD^4? Is that no use-case?

Finii commented 7 months ago

I also think the globbing GitRepository::add(...) should have at least one unit test which shows that and how globbing works.

Added test. Fixed reset(n). Docs coming soon (after baking Christmas cookies :-} ).

Finii commented 7 months ago

There was a conflict with main, fixed and rebase -> force push.

Finii commented 7 months ago

Rebase on master after that merged the refactoring; force push.

Finii commented 7 months ago

In the output operator I called the repo state RepoState. The type that holds it is named thus: using RepoState = std::vector<FileStatus>;

The function to get the repo state is called status() which is kind of a misnomer because that is the repo state you get back and not the repo status. Probably we should rename the function too. I did not do that yet to raise this discussion.

Also collect_status() should be collect_state() or collect_statuses().

note I also considered making RepoState a true type and not just an alias, which does have some benefits, but as it is in fact just a std::vector and inheriting that (to get a nice own type with empty base optimization) that would not be advisable (see Core Guidelines) and the vector as a data member would need accessors and such, and the handling will become much more convoluted.

Finii commented 7 months ago

Sorry, again force push, reordered the tests to make more sens, building from simple to complex.

Finii commented 7 months ago

Solving merge conflict after pulling the other PR into main, force push