uptech
Make the `rr+` state actually mean what it was intended to mean
The rr+ state was intended to mean that you had made changes to that patch since you last requested review of it. This was intended to be an indicator vs rr that reminds you that you need to re-request review for that patch.
The current implementation of this is a poor mans implementation that simply compares the sha of the patch when the last request for review was made against the current sha of the patch. If those are the same then it appears as rr. If they are different then it appears as rr+. The downside of this approach is that we see rr+ after a rebase or reording of patches which we shouldn't.
This is because the sha of a commit includes the value of the reference to its parent commit.
So one approach would be to build our own hash of a commit that hashes everything except for the references to other commits. We could then store this hash when we do a request for review and recompute it and compare the newly computed value against the stored value to see if we should present an rr+ or an rr. This would then make the rr+ indicator actually represent what we indend and truely be useful.
This is an example of how the normal commit sha is generated
git cat-file -p 01582a | git hash-object -t $(git cat-file -t 01582a) --stdin
The git cat-file -p dumps just the description and attributes not that actual content of the commit body. So I am guessing that is represnted by the tree object sha1. So need to investigate that further and if that changes as a commit is moved around the git tree.
Sadly it looks like we can't use the normal process of using cat-file -p because the tree sha changes when the patches are simply reordered. You can see this in the example run through below.
So we might have to figure out two separate commands. One that gives us the attributes we care about and one that gives us the body of the commit. Allowing us to then combine them in a consistent format and compute a resulting hash.
✔ git ps ls 3.0.2 89m main b27a884 ✗
4 b27a88 Add catFilePretty(ref:) to GitShell
3 rr 579c9c Add git-ps co <patch-index> subcommand
2 rr+ f3241b Add function to parse checkout subcommand
1 rr+ 8945b0 Add help messaging for git-ps co
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
If I cat-file the "Add help messaging for git-ps co" patch in this position. I get the following.
✔ git cat-file -p 8945b0 3.0.2 89m main b27a884 ✗
tree be5ff5b7d723383b17b6ec28556eb8c7936fcf3b
parent 01582a68042ae126fa602d3dce7aa5eb88f9e302
author Andrew De Ponte <[email protected]> 1631822206 -0400
committer Andrew De Ponte <[email protected]> 1631823746 -0400
Add help messaging for git-ps co
So that we can use it later to implement the actually `git-ps co
<patch-index>` command and have useful help messaging.
This relates to issue #40.
ps-id: 44B199C9-6AAA-4DDF-AA48-1CD32ED9B4E5
If I then reorder the patches by doing a git ps rebase and reorder the stack to the following:
✔ git ps ls 3.0.2 93m main 3c92c54
4 3c92c5 Add catFilePretty(ref:) to GitShell
3 rr+ 525f98 Add git-ps co <patch-index> subcommand
2 rr+ ff4857 Add help messaging for git-ps co
1 rr+ bea357 Add function to parse checkout subcommand
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
Then if I cat-file of the "Add help messaging for git-ps co" patch I get the following:
✔ git cat-file -p ff4857 3.0.2 94m main 3c92c54
tree 9013cd8bbebb29c573dbacc102b762d95d9e3196
parent bea357e3db2175e32eb2cb81139524c0d6c125f0
author Andrew De Ponte <[email protected]> 1631822206 -0400
committer Andrew De Ponte <[email protected]> 1631832691 -0400
Add help messaging for git-ps co
So that we can use it later to implement the actually `git-ps co
<patch-index>` command and have useful help messaging.
This relates to issue #40.
ps-id: 44B199C9-6AAA-4DDF-AA48-1CD32ED9B4E5
Now we need to investigate the git diff output to see if the diff changes as we rebase.
✔ git ps ls 3.0.2 125m main 3c92c54 ✗
4 3c92c5 Add catFilePretty(ref:) to GitShell
3 rr+ 525f98 Add git-ps co <patch-index> subcommand
2 rr+ ff4857 Add help messaging for git-ps co
1 rr+ bea357 Add function to parse checkout subcommand
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
✔ git diff --no-color -p ff4857^ ff4857 3.0.2 126m main 3c92c54 ✗
diff --git a/Sources/GitPatchStackCore/GitPatchStack.swift b/Sources/GitPatchStackCore/GitPatchStack.swift
index b9346c8..d66df2f 100644
--- a/Sources/GitPatchStackCore/GitPatchStack.swift
+++ b/Sources/GitPatchStackCore/GitPatchStack.swift
@@ -775,6 +775,7 @@ public final class GitPatchStack {
Commands:
ls List the stack of patches
show <i> Show the patch diff and details
+ co <i> Checkout the specified patch in the stack
pull Fetch the state of origin/master and rebase the stack of patches onto it
rebase Interactive rebase the stack of patches
rr <i> Request review of the patch or update existing request to review
@@ -805,6 +806,17 @@ func showSubCommandHelpText() -> String {
"""
}
+func checkoutSubCommandHelpText() -> String {
+ return """
+ usage: git-ps co <patch-index> [-h | --help]
+
+ Checkout the patch identified by the given patch-index. This is
+ useful especially when you want to go headless to ignore some
+ patches higher up in the stack and test something.
+
+ """
+}
+
func requestReviewSubCommandHelpText() -> String {
return """
usage: git-ps rr (<patch-index> | <start-patch-index>-<end-patch-index>) [-n <branch>]
Then if we rebase and move the "Add help messaging for git-ps co" patch around and then look we get the following.
✔ git ps ls 3.0.2 129m main cdacbb0
4 cdacbb Add catFilePretty(ref:) to GitShell
3 rr+ 96b778 Add help messaging for git-ps co
2 rr+ 48b699 Add git-ps co <patch-index> subcommand
1 rr+ bea357 Add function to parse checkout subcommand
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
✔ git diff --no-color -p 96b778^ 96b778 3.0.2 129m main cdacbb0
diff --git a/Sources/GitPatchStackCore/GitPatchStack.swift b/Sources/GitPatchStackCore/GitPatchStack.swift
index 6906ceb..874f222 100644
--- a/Sources/GitPatchStackCore/GitPatchStack.swift
+++ b/Sources/GitPatchStackCore/GitPatchStack.swift
@@ -780,6 +780,7 @@ public final class GitPatchStack {
Commands:
ls List the stack of patches
show <i> Show the patch diff and details
+ co <i> Checkout the specified patch in the stack
pull Fetch the state of origin/master and rebase the stack of patches onto it
rebase Interactive rebase the stack of patches
rr <i> Request review of the patch or update existing request to review
@@ -810,6 +811,17 @@ func showSubCommandHelpText() -> String {
"""
}
+func checkoutSubCommandHelpText() -> String {
+ return """
+ usage: git-ps co <patch-index> [-h | --help]
+
+ Checkout the patch identified by the given patch-index. This is
+ useful especially when you want to go headless to ignore some
+ patches higher up in the stack and test something.
+
+ """
+}
+
func requestReviewSubCommandHelpText() -> String {
return """
usage: git-ps rr (<patch-index> | <start-patch-index>-<end-patch-index>) [-n <branch>]
So it looks like the index line of the diff changes depending on where the patch is in the stack. So we will need to strip that out as well.
So, I think what makes the most sense is to use git show --no-color --pretty=raw <sha> to get both the attributes as well as the diff and then we can just filter out any line that starts with commit , tree , parent , or index . Once we have that output we should just be able to hash the contents and get a hash value that will stay consistent irrespective of where the patch is in the stack, but should change when the actually diff or attributes of the patch change.
I whipped up an experiment real quick to validate this. See the results following:
✔ git ps ls 3.0.2 58m main afa84d2 ✗
DREW: contenxt-hash: de2a9f6644229bd38b7d57ea321eb31d718069fb
4 afa84d Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: 2a9719f840d8cc391662332707aac46768b837b7
3 rr+ a82393 Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: ae2a35b9e9126a846dc198c9fac3c51595be8f18
2 rr+ e342b3 Add function to parse checkout subcommand
DREW: contenxt-hash: 54effd21ca7ebf34b8cec083c824cfe76641d788
1 rr+ cb149b Add help messaging for git-ps co
DREW: contenxt-hash: 99b4c60ba46d96cda94f8163b3bc6a535d242976
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
Now if we change the order of the patches on the stack.
DREW: contenxt-hash: a49760d5dafd66b50b9daf36a44f3aec0401b9c3
4 1318da Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: c796339ada9a9d22547fbb92162438de9863cc0e
3 rr+ a91d81 Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: db862d9167d3fa7748618d459b7532eeea7c974f
2 rr+ 430b79 Add help messaging for git-ps co
DREW: contenxt-hash: 7c711c1df9bded9864171263a10ae7b64fc56f37
1 rr+ 56ef23 Add function to parse checkout subcommand
DREW: contenxt-hash: 99b4c60ba46d96cda94f8163b3bc6a535d242976
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
Looking at the results we can see that the printed context-hash for the "Add help messaging for git-ps co" patch is as follows:
DREW: contenxt-hash: 54effd21ca7ebf34b8cec083c824cfe76641d788
1 rr+ cb149b Add help messaging for git-ps co
DREW: contenxt-hash: db862d9167d3fa7748618d459b7532eeea7c974f
2 rr+ 430b79 Add help messaging for git-ps co
So it turns out that it is NOT consistent when we move rebase and move the patch for some reason that I don't understand off hand. Will have to investigate that further. It is consistent across git ps ls runs as long as we don't change the order of the patches.
Better testing shows that we are close to having a sha1 we can use.
✔ git ps ls 3.0.2 127m main d974265
DREW: contenxt-hash: 6befa9c5e56dd8e0ca06ff90914dcac0f480a33f
4 d97426 Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: 337e48ec7545acb910937eb4b929b728a643b275
3 rr+ ba65f8 Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: 1b07dd09912d16eb8afcef1422e963cf00a11c43
2 rr+ a4738e Add function to parse checkout subcommand
DREW: contenxt-hash: 962e704f6a3d3179c3332fde52fe48b95a4c5d0d
1 rr+ 240093 Add help messaging for git-ps co
DREW: contenxt-hash: 99b4c60ba46d96cda94f8163b3bc6a535d242976
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
~/code/uptech/git-ps
✔ ls -l 3.0.2 128m main d974265
total 120
-rw-r--r-- 1 adeponte staff 2266 Feb 17 2021 CHANGELOG.md
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 GitPatchStackCoreTests
-rw-r--r-- 1 adeponte staff 1084 Feb 22 2020 LICENSE
-rw-r--r-- 1 adeponte staff 256 Feb 22 2020 Makefile
-rw-r--r-- 1 adeponte staff 3099 Dec 10 2020 OLD_CHANGELOG.md
-rw-r--r-- 1 adeponte staff 892 Sep 16 10:10 Package.resolved
-rw-r--r-- 1 adeponte staff 1559 May 11 18:22 Package.swift
-rw-r--r-- 1 adeponte staff 9294 Jun 9 16:20 README.md
-rw-r--r-- 1 adeponte staff 1308 Mar 30 2020 RESOURCES.md
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 Sources
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 Tests
-rw-r--r-- 1 adeponte staff 1167 Sep 16 21:52 foo-01582a68042ae126fa602d3dce7aa5eb88f9e302.txt
-rw-r--r-- 1 adeponte staff 1712 Sep 16 21:52 foo-24009309e4517c10fea3322d89e455edf0540e08.txt
-rw-r--r-- 1 adeponte staff 1516 Sep 16 21:52 foo-a4738e2ef65f65c80fac87753f2368b9a8a161c3.txt
-rw-r--r-- 1 adeponte staff 3242 Sep 16 21:52 foo-ba65f8f25a152a4df1ed327ce05267d58cd54bd8.txt
-rw-r--r-- 1 adeponte staff 2128 Sep 16 21:52 foo-d974265b88a8cb9f6bf78ac2a82099a2448e5918.txt
drwxr-xr-x@ 8 adeponte staff 256 Feb 21 2020 git-ps.xcodeproj
drwxr-xr-x 3 adeponte staff 96 Feb 22 2020 presentations
DREW: contenxt-hash: c75be470fd87122ccb9f59716a2509560103e7fc
4 c22154 Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: 10fab130742d3d0d7c52b42f56206b44519dc6f0
3 rr+ d033fc Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: 538b8777c055e8ccdafb18d5c5e78718881dbf0f
2 rr+ 1ad02f Add help messaging for git-ps co
DREW: contenxt-hash: 14ebf3497b8ae3025af18155050a784b45987ef0
1 rr+ 47533a Add function to parse checkout subcommand
DREW: contenxt-hash: 99b4c60ba46d96cda94f8163b3bc6a535d242976
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
~/code/uptech/git-ps
✔ ls -l 3.0.2 129m main c22154e
total 152
-rw-r--r-- 1 adeponte staff 2266 Feb 17 2021 CHANGELOG.md
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 GitPatchStackCoreTests
-rw-r--r-- 1 adeponte staff 1084 Feb 22 2020 LICENSE
-rw-r--r-- 1 adeponte staff 256 Feb 22 2020 Makefile
-rw-r--r-- 1 adeponte staff 3099 Dec 10 2020 OLD_CHANGELOG.md
-rw-r--r-- 1 adeponte staff 892 Sep 16 10:10 Package.resolved
-rw-r--r-- 1 adeponte staff 1559 May 11 18:22 Package.swift
-rw-r--r-- 1 adeponte staff 9294 Jun 9 16:20 README.md
-rw-r--r-- 1 adeponte staff 1308 Mar 30 2020 RESOURCES.md
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 Sources
drwxr-xr-x 4 adeponte staff 128 Feb 22 2020 Tests
-rw-r--r-- 1 adeponte staff 1167 Sep 16 21:54 foo-01582a68042ae126fa602d3dce7aa5eb88f9e302.txt
-rw-r--r-- 1 adeponte staff 1712 Sep 16 21:54 foo-1ad02f828215d64922930bd80e7e36eb677f2257.txt
-rw-r--r-- 1 adeponte staff 1712 Sep 16 21:52 foo-24009309e4517c10fea3322d89e455edf0540e08.txt
-rw-r--r-- 1 adeponte staff 1516 Sep 16 21:54 foo-47533af9fa0d98f3d9a456af1e321834b9fda31d.txt
-rw-r--r-- 1 adeponte staff 1516 Sep 16 21:52 foo-a4738e2ef65f65c80fac87753f2368b9a8a161c3.txt
-rw-r--r-- 1 adeponte staff 3242 Sep 16 21:52 foo-ba65f8f25a152a4df1ed327ce05267d58cd54bd8.txt
-rw-r--r-- 1 adeponte staff 2128 Sep 16 21:54 foo-c22154e62e52b9ed43fa729c8460a5341a4a7a0d.txt
-rw-r--r-- 1 adeponte staff 3242 Sep 16 21:54 foo-d033fc32ca8b76d6b127a0e93ce3b59fbc7ce527.txt
-rw-r--r-- 1 adeponte staff 2128 Sep 16 21:52 foo-d974265b88a8cb9f6bf78ac2a82099a2448e5918.txt
drwxr-xr-x@ 8 adeponte staff 256 Feb 21 2020 git-ps.xcodeproj
drwxr-xr-x 3 adeponte staff 96 Feb 22 2020 presentations
Some testing reveals that we almost have it. Turns out that ther is a timestamp encoding in the same line as the commiter we have to some how deal with.
✔ diff foo-1ad02f828215d64922930bd80e7e36eb677f2257.txt foo-24009309e4517c10fea3322d89e455edf0540e08.txt 3.0.2 129m main c22154e
2c2
< committer Andrew De Ponte <[email protected]> 1631843643 -0400
---
> committer Andrew De Ponte <[email protected]> 1631841846 -0400
If we also ignore the committer line then we can see that it is working. I will have to think about the committer being ignored. Maybe it is ok for committer to be ignored as long as author isn't ignored.
✔ git ps ls 3.0.2 141m main c22154e
DREW: contenxt-hash: b456404316d863e83523ce8b9e6dcec3d25b5f92
4 c22154 Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: 58a2d642f11990ef70f2241da3061121f476d238
3 rr+ d033fc Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: 15d424d5a12604023feb3ecf7ed56be2937bfd7a
2 rr+ 1ad02f Add help messaging for git-ps co
DREW: contenxt-hash: c089e1ac664a00cd53ff1379b559da04d6d4a2bc
1 rr+ 47533a Add function to parse checkout subcommand
DREW: contenxt-hash: 53c94437437a30ce6970e32f86f7d8b4f89fe418
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
✔ git ps ls 3.0.2 141m main e7d6946
DREW: contenxt-hash: b456404316d863e83523ce8b9e6dcec3d25b5f92
4 e7d694 Add getShowNoColor(ref:) to GitShell
DREW: contenxt-hash: 58a2d642f11990ef70f2241da3061121f476d238
3 rr+ f6a0ab Add git-ps co <patch-index> subcommand
DREW: contenxt-hash: c089e1ac664a00cd53ff1379b559da04d6d4a2bc
2 rr+ 309aa3 Add function to parse checkout subcommand
DREW: contenxt-hash: 15d424d5a12604023feb3ecf7ed56be2937bfd7a
1 rr+ 63a9cb Add help messaging for git-ps co
DREW: contenxt-hash: 53c94437437a30ce6970e32f86f7d8b4f89fe418
0 rr+ 01582a Add checkout(patchIndex:) to GitPatchStack class
