Skip to content

Conversation

@vr8hub
Copy link

@vr8hub vr8hub commented Jun 18, 2020

Undo currently does not work when there is only a single commit. There are instances when doing a soft undo on a repository with only a single commit is desired.
In implementing that, I added a few parameter checks, including:

  • Cannot specify an undo count greater than the commit count.
  • Cannot hard undo all commits, e.g. if commit count is 3, can't hard undo all 3.
  • Catch bad parameters, i.e. something other than -s/--soft/-h/--hard as the first parameter or a non-numeric as the second parameter.

Tested on OSX 10.14.6, which is all I have available. I don't believe I used anything that isn't portable, however. (Famous last words.)

bin/git-undo Outdated
@@ -1,6 +1,28 @@
#!/usr/bin/env bash

back="^"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The $back is unused now. Please remove it too.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, should have caught that. Thanks.

bin/git-undo Outdated
type=${1:-soft}
undo_cnt=${2:-1}
reset=${3:-""}
echo "type=$type, cnt=$undo_cnt, reset=$reset"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the debug log.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh! I looked at that right before my last round of testing and said, "Don't forget to get rid of that!" and promptly didn't get rid of that.

git reset --hard HEAD$back
exit $?
parm1=hard
parm2=${2:-1}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use more reasonable argument name than parmX.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kept them generic on purpose. The names in the function are what matter. If you would like to name the parameters, I'll be glad to use them.

bin/git-undo Outdated
;;
esac

if [[ ! $parm2 =~ [0-9]*([0-9]) ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be [1-9]* instead?

bin/git-undo Outdated
echo "type=$type, cnt=$undo_cnt, reset=$reset"
if [ $undo_cnt -gt $ccnt ]; then
echo "Only $ccnt $cstr, cannot undo $undo_cnt"
elif [ "$type" = "hard" -a $ccnt -eq $undo_cnt ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to use [ "$type" = "hard" ] && [ $ccnt -eq $undo_cnt ], which is more clear.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I disagree that it's clearer, but it's not a hill worth dying on, so I'll change it.

bin/git-undo Outdated
echo "Only $ccnt $cstr, cannot undo $undo_cnt"
elif [ "$type" = "hard" -a $ccnt -eq $undo_cnt ]; then
echo "Cannot hard undo all commits"
elif [ "$type" = "soft" -a $ccnt -eq 1 ]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

@vr8hub
Copy link
Author

vr8hub commented Jun 19, 2020

Fixed everything but the parmX names. Again, I named them that way on purpose, but if you have suggestions I'll be glad to use them instead.

bin/git-undo Outdated
;;
esac

if [[ ! $parm2 =~ [1-9]*([0-9]) ]]; then
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have changed my mind. Since 0 is not a valid count, maybe the pattern should be [1-9][0-9]*?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not really changing your mind, that's what you brought up initially, which is why I changed the first one to be 1-9 instead of 0-9. What I have does exactly what you're suggesting, using [[ regex syntax.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vr8hub
Err... This regex pattern is better: if [[ ! $parm2 =~ ^[1-9][0-9]*$ ]].
It can detect all three invalid cases:0, 1s, 01.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@spacewander spacewander merged commit 5a79873 into tj:master Jun 21, 2020
@spacewander
Copy link
Collaborator

@vr8hub
Merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants