feat: refs support pseudo refs

[orthros]

Adds support in the references iter Platform to iterate over the pseudo references in the repository.

Note that the all funciton will not return pseudo references to maintain backwards compatibility.

[Byron]

Thanks for contributing this and I see how it can be useful!

I set it back to draft as there are no tests which are strictly required. Is this something you'd want to continue on?

If so, I have some additional notes below.
And after writing them I think that iter_pseudo_refs() should better be its own iterator to avoid all these max_depth related changes entirely.

[Byron]

This should go into a separate commit right after the one changing gix-ref conventionally saying something like adapt to changes in gix-features.

[Byron]

Since this is tuned to pick up FETCH_HEAD and HEAD only, I wonder if there is anything else this would have to do?

If not, would it not be better to make iter_pseudo_refs() its own iterator instead of shoehorning it in?

[orthros]

This will pick up more than just FETCH_HEAD and HEAD. There are other tools (e.g. jiri) which create pseudo refs (JIRI_HEAD) as a part of their tooling and workflow.

Just to make sure I understand what you're suggesting, You suggest that Platform::pseudo_refs() returns Result<PseudoRefIter<'_>> as opposed to Iter<'_> where PseudoRefIter<'_> impls Iterator<Item = Result<gix::Reference<'r>, Box<dyn std::error::Error + Send + Sync + 'static) just like Iter<'_>?

If that's the case, I can see that being fine, however, that would make consumers of the Platform a little bit clunky as now any function

fn print_them(repo: &Repository) -> Result<()> {
  print_some_refs(repo.references()?.pseudo_refs());
  print_some_refs(repo.references()?.all());
}

fn print_some_refs(refs: gix::reference::iter::Iter<'_>) {
  for ref in refs {
    println!("{:?}", ref);
  }
}

has to be re-written like so

fn print_them(repo: &Repository) -> Result<()> {
  print_some_refs(repo.references()?.pseudo_refs());
  print_some_refs(repo.references()?.all());
}

fn print_some_refs<'r>(refs: impl Iterator<Item = Result<gix::Reference<'r>, Box<dyn  std::error::Error + Send + Sync + 'static) {
  for ref in refs {
    println!("{:?}", ref);
  }
}

Which isn't necessarily "wrong" but that smells a bit clunkier to me.

I'm curious about your thoughts.

[orthros]

I believe I addressed your concerns. PLMK what you think!

[Byron]

Thanks so much!

I believe I addressed your concerns. PLMK what you think!

First of all, apologies for not responding to the comment above - I somehow missed it, which might just mean that I archived the email without actually acting on it, which was my intent.
This makes it even more unfortunate that what I wanted to say is along the lines of "I see what you mean, let's stick to keeping the SortedLoosePaths version", instead of creating a separate iterator.

Would you be willing to roll that back?

If you are out of time, just let me know and I will do it. Please note that I will have to rewrite your commits though as the commit-separation isn't addressed yet. (The 'adapt' commits are missing, and as a rule of thump here, changes to a single crate have to go into their own commits).

[Byron]

Further, it seems like git2 lists pseudo-refs as well when listing all references (there seems to be no differentiation in listing methods), and it's something that gitoxide should also consider.

With the current iterator setup it's not super trivial to do in gix-refs, but gix could probably build something around it and instead do two iterations if it wanted to. I wouldn't be sure if all() should be changed though, and am very sure that this doesn't have to be part of this PR. In any case, if there is interest from your side, it's something we could tackle in a follow-up.

[orthros]

If I had a dollar for every time I accidentally archived an email I meant to respond to I'd be retired!

Totally fine rolling it back! I won't be able to do so until early next week.

Also I see I split the adapt commit into one but not the other.

IIRC this broke the commit from compiling. Is that acceptable to have a non compiling commit in the chain?

[Byron]

Thanks so much!

IIRC this broke the commit from compiling. Is that acceptable to have a non compiling commit in the chain?

Yes, that's the trade-off to get changelog entries that fit for the respective crate. Otherwise, say, a breaking change would cause two crates to get a minor version bump, the one that actually broke, and the one that just adapted to the breaking change in a private API, along with changelog entries for both.

Totally fine rolling it back! I won't be able to do so until early next week.

No problem, I am looking forward to getting this merged.