gix-revision: Parse revisions with the @ character

[lthms]

Fixes #802

Happy to have some feedback. The solution seems to work, and makes sense to me, but of course I’m not very familiar with this codebase 😅

---

Ok for Byron review the PR on video?

• I give my permission to record review and upload on YouTube publicly

If I think the review will be helpful for the community, then I might record and publish a video.

[Byron]

Thanks a lot for your contribution, I never thought about @ being anything else than a special character, and it shows.

And guess who also it not familiar with the codebase (anymore) 😅?

Thus it's time to trust the tests, and I did manage to find more breaking examples by using ranges which might indicate that the right spot for handling @ characters is at another place, or ranges also have to be handled specifically.

I am still looking into it to see what to do here.

[Byron]

Ranges now work as well and in theory, this passes the fuzzer as well (practical to have it, thanks google-fuzz-project!). If so, and you can't find any other cases maybe by experimenting with gix rev parse -e <revspec> this should be good to go.

[lthms]

Thanks for the refactoring! Looks like the CI does not pass actually! The output is a bit cryptic to me, but I will try to have a look.

[lthms]

Looks like reintroducing the error for things like .@. is necessary.

[Byron]

No worries about it. The idea is that the fuzzer finds an input that triggers a panic, then a test can be added that verifies that the panic doesn't occour anymore.

I'll be adding the test shortly.

[Byron]

Yes, it's my first time the fuzzer found something in this CI and then the artifact can be downloaded from GitHub directly. I have added a test with that input, and if it goes through I'd think the implementation should be fine.

Thanks again for getting the ball rolling on this one!

[lthms]

You are very welcome! I guess it might make sense to squash the two commits together, and mark both of us as co-authors. (:

Do you mind doing a (minor?) release for gix-revision? So that I can push a fix to Stacked Git too?
(it is not that urgent, I can keep use my fixed version of stacked git built locally, but it would be nice to fix this regression at some point)

[Byron]

You are very welcome! I guess it might make sense to squash the two commits together, and mark both of us as co-authors. (:

A good point! It's done locally (don't want to trigger CI again), and it will be merged in instead of what's currently here.

Do you mind doing a minor release for gix-revision? So that I can push a fix to Stacked Git too?

That's the plan - I will post here when done in a bit.

[Byron]

The PR was brought into main and a new gix-revision patch release make the fix available.
CC @jpgrayson in case this warrants a new stg release.

[lthms]

Thanks again @Byron. For the record, I was meaning to squash both our commits, not just claim co-authorship on you own 😅 but anyway, it’s nice to see this into main this quickly.