Ensure that `grc` and `gcp` always have the correct ordering, regardless of how they were input

cjappl commented 1 year ago

Check list

[x] I have performed a self-review of my code
[x] I have commented my code in hard-to-understand areas
[x] I have made corresponding changes to the documentation

Description

Closes #253. Essentially the idea is this:

When cherry picking a group of commits, most of the time you want to cherry pick oldest to newest. This means you have the lowest chance of having a conflict or error
when reverting a commit. You most often want to revert from newest to oldest. Similarly prevents errors

This commit does that, based on the wonderful suggestion on how to do that from @carlfriedrich in the linked ticket (#253).

Best reviewed with turning whitespace off, as I added some in there. I also made a few small stylistic changes so these two functions mirrored each other more closely, as fundamentally they do similar things (get a list of commits, and feed them to the function).

TO TEST:

Try to revert one commit
Try to revert 3 commits in all orders - the resulting order should be the same, from newest to oldest
Try to cherry pick one commit
Try to cherry pick 3 commits in all orders - the resulting order should always be the same, from oldest to newest

I was also adding a line to confirm this, which may be helpful in your testing:

echo "${commits[@]}"

on lines 251 and 247.

Type of change

[x] Bug fix
[x] New feature

Test environment

Shell
- [ ] bash
- [x] zsh
- [x] fish
OS
- [x] Mac OS X

cjappl commented 1 year ago

Calling all the usual suspects @sandr01d @carlfriedrich @wfxr to test if you have time before this is merged. Works like a charm on my end so far and removes one of my biggest pain points!

cjappl commented 1 year ago

Just used this for the first time in my work day for gcp man is it nice!! I've been using a...shudder...GUI to do cherry picking because of this issue and this may save me from ever doing it again!

cjappl commented 1 year ago

Added comments. Will keep this open for a few days if others want to review/poke around. I'll merge it early next week if I hear no objections. Thanks for the review @carlfriedrich

sandr01d commented 1 year ago

Works great, good work! I've requested a minor change above though.

cjappl commented 1 year ago

Works great, good work! I've requested a minor change above though.

@sandr01d could you re-post your request? I don't see it anywhere

sandr01d commented 1 year ago

Works great, good work! I've requested a minor change above though.

@sandr01d could you re-post your request? I don't see it anywhere

You've already addressed it in 1eac221 :slightly_smiling_face:

cjappl commented 1 year ago

Ah! Nice. Good teamwork then :) I figured it out via magic.

I’ll merge this sometime soon. Thanks for the eyes.

On Thu, Dec 15, 2022 at 12:32 PM, sandr01d @.***> wrote:

Works great, good work! I've requested a minor change above though.

@.***(https://github.com/sandr01d) could you re-post your request? I don't see it anywhere

You've already addressed it in 1eac221 🙂

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

wfxr / forgit