[PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

This implements `tg patch -i` and `tg patch -w` to see current patch as
generated against not-yet-committed index and worktree.


NOTE: unfortunately `git cat-file blob <file>` does not provide an option
to cat file from worktree (only from an object or from index), so I had to
unroll my own `cat file topic:file` with special support for '(i)' and
'(w)' topics.

Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

---
 README                     |    5 +++--
 contrib/tg-completion.bash |    6 ++++++
 tg-patch.sh                |   31 +++++++++++++++++++++++++------
 tg.sh                      |   21 +++++++++++++++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/README b/README
index 1d38365..5796112 100644
--- a/README
+++ b/README
@@ -284,8 +284,9 @@ tg patch
 	tg patch will be able to automatically send the patches by mail
 	or save them to files. (TODO)
 
-	TODO: tg patch -i to base at index instead of branch,
-		-w for working tree
+	Options:
+	  -i		base patch generation on index instead of branch
+	  -w		base patch generation on working tree instead of branch
 
 tg mail
 ~~~~~~~
diff --git a/contrib/tg-completion.bash b/contrib/tg-completion.bash
index 9641d04..de8a7b5 100755
--- a/contrib/tg-completion.bash
+++ b/contrib/tg-completion.bash
@@ -359,6 +359,12 @@ _tg_patch ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 
 	case "$cur" in
+	-*)
+		__tgcomp "
+			-i
+			-w
+		"
+		;;
 	*)
 		__tgcomp "$(__tg_topics)"
 	esac
diff --git a/tg-patch.sh b/tg-patch.sh
index dc699d2..db1ad09 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -5,14 +5,25 @@
 
 name=
 
+topic=
+diff_opts=
+diff_committed_only=yes	# will be unset for index/worktree
+
 
 ## Parse options
 
 while [ -n "$1" ]; do
 	arg="$1"; shift
 	case "$arg" in
+	-i)
+		topic='(i)'
+		diff_opts="$diff_opts --cached";;
+		diff_committed_only=;
+	-w)
+		topic='(w)'
+		diff_committed_only=;;
 	-*)
-		echo "Usage: tg [...] patch [NAME]" >&2
+		echo "Usage: tg [...] patch [-i | -w] [NAME]" >&2
 		exit 1;;
 	*)
 		[ -z "$name" ] || die "name already specified ($name)"
@@ -20,31 +31,39 @@ while [ -n "$1" ]; do
 	esac
 done
 
+
+[ -n "$name"  -a  -z "$diff_committed_only" ]  &&
+	die "-i/-w are mutually exclusive with NAME"
+
 [ -n "$name" ] || name="$(git symbolic-ref HEAD | sed 's#^refs/\(heads\|top-bases\)/##')"
 base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)" ||
 	die "not a TopGit-controlled branch"
 
+# if not index/worktree, topic is current branch
+[ -z "$topic" ] && topic="$name"
+
+
 
 setup_pager
 
-git cat-file blob "$name:.topmsg"
+cat_file "$topic:.topmsg"
 echo
-[ -n "$(git grep '^[-]--' "$name" -- ".topmsg")" ] || echo '---'
+[ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
 
 # Evil obnoxious hack to work around the lack of git diff --exclude
 git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
-git diff-tree --name-only "$base_rev" "$name" |
+git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
 	fgrep -vx ".topdeps" |
 	fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
 if [ -s "$git_is_stupid" ]; then
-	cat "$git_is_stupid" | xargs git diff --patch-with-stat "$base_rev" "$name" --
+	cat "$git_is_stupid" | xargs git diff --patch-with-stat $diff_opts "$base_rev" ${diff_committed_only:+"$name"} --
 else
 	echo "No changes."
 fi
 rm "$git_is_stupid"
 
 echo '-- '
-echo "tg: ($base_rev..) $name (depends on: $(git cat-file blob "$name:.topdeps" | paste -s -d' '))"
+echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
 branch_contains "$name" "$base_rev" ||
 	echo "tg: The patch is out-of-date wrt. the base! Run \`$tg update\`."
 
diff --git a/tg.sh b/tg.sh
index b64fc3a..ab0db52 100644
--- a/tg.sh
+++ b/tg.sh
@@ -17,6 +17,27 @@ die()
 	exit 1
 }
 
+# cat_file "topic:file"
+# Like `git cat-file blob $1`, but topics (i) and (w) means index and worktree
+cat_file()
+{
+	arg="$1"
+	case "$arg" in
+	'(w):'*)
+		arg=$(echo "$arg" | tail --bytes=+5)
+		cat "$arg"
+		return
+		;;
+	'(i):'*)
+		# ':file' means cat from index
+		arg=$(echo "$arg" | tail --bytes=+5)
+		git cat-file blob ":$arg"
+		;;
+	*)
+		git cat-file blob "$arg"
+	esac
+}
+
 # setup_hook NAME
 setup_hook()
 {
-- 
tg: (a3a5be1..) t/tg-patch-worktree (depends on: t/tg-patch-setup-pager)

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 1072 bytes --]

also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.09.0722 +1300]:
> This implements `tg patch -i` and `tg patch -w` to see current
> patch as generated against not-yet-committed index and worktree.

I think at this early stage, it would make sense to use long options
and not reserve short options yet. Unless Petr disagrees, I'd kindly
ask you to use long options instead. Once TopGit has been around for
a while, we can provide short options for the most important long
options.

This is possibly too conservative, but I've been bitten by lack of
new letters before because I've used them all up for options that
later turned out not to be needed.

I have not yet had the time to actually look at the patch.

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"when zarathustra was alone... he said to his heart: 'could it be
 possible! this old saint in the forest hath not yet heard of it, that
 god is dead!'"
                                                 - friedrich nietzsche
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

On Fri, Jan 09, 2009 at 08:53:56AM +1300, martin f krafft wrote:
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.09.0722 +1300]:
> > This implements `tg patch -i` and `tg patch -w` to see current
> > patch as generated against not-yet-committed index and worktree.
> 
> I think at this early stage, it would make sense to use long options
> and not reserve short options yet. Unless Petr disagrees, I'd kindly
> ask you to use long options instead. Once TopGit has been around for
> a while, we can provide short options for the most important long
> options.
> 
> This is possibly too conservative, but I've been bitten by lack of
> new letters before because I've used them all up for options that
> later turned out not to be needed.

I agree, but when I found myself needing something like
`tg patch --index`, I've spot this in README:


    --- a/README
    +++ b/README
    @@ -284,8 +284,9 @@ tg patch
            tg patch will be able to automatically send the patches by mail
            or save them to files. (TODO)
    
    -       TODO: tg patch -i to base at index instead of branch,
    -               -w for working tree

So I concluded -i/-w was planned from the beginning.


I myself would call these options --index and --work or something
like that, but I'll be ok with any option.


Thanks,
Kirill

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

I'm sorry, but I've found a mistake in my code for case:

diff --git a/tg-patch.sh b/tg-patch.sh
index db1ad09..d701c54 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -17,8 +17,8 @@ while [ -n "$1" ]; do
        case "$arg" in
        -i)
                topic='(i)'
-               diff_opts="$diff_opts --cached";;
-               diff_committed_only=;
+               diff_opts="$diff_opts --cached";
+               diff_committed_only=;;
        -w)
                topic='(w)'
                diff_committed_only=;;


So here is corrected patch:


From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
To: Petr Baudis <pasky@suse.cz>
Cc: martin f krafft <madduck@madduck.net>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

This implements `tg patch -i` and `tg patch -w` to see current patch as
generated against not-yet-committed index and worktree.


NOTE: unfortunately `git cat-file blob <file>` does not provide an option
to cat file from worktree (only from an object or from index), so I had to
unroll my own `cat file topic:file` with special support for '(i)' and
'(w)' topics.

Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

---
 README                     |    5 +++--
 contrib/tg-completion.bash |    6 ++++++
 tg-patch.sh                |   31 +++++++++++++++++++++++++------
 tg.sh                      |   21 +++++++++++++++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/README b/README
index 1d38365..5796112 100644
--- a/README
+++ b/README
@@ -284,8 +284,9 @@ tg patch
 	tg patch will be able to automatically send the patches by mail
 	or save them to files. (TODO)
 
-	TODO: tg patch -i to base at index instead of branch,
-		-w for working tree
+	Options:
+	  -i		base patch generation on index instead of branch
+	  -w		base patch generation on working tree instead of branch
 
 tg mail
 ~~~~~~~
diff --git a/contrib/tg-completion.bash b/contrib/tg-completion.bash
index 9641d04..de8a7b5 100755
--- a/contrib/tg-completion.bash
+++ b/contrib/tg-completion.bash
@@ -359,6 +359,12 @@ _tg_patch ()
 	local cur="${COMP_WORDS[COMP_CWORD]}"
 
 	case "$cur" in
+	-*)
+		__tgcomp "
+			-i
+			-w
+		"
+		;;
 	*)
 		__tgcomp "$(__tg_topics)"
 	esac
diff --git a/tg-patch.sh b/tg-patch.sh
index dc699d2..d701c54 100644
--- a/tg-patch.sh
+++ b/tg-patch.sh
@@ -5,14 +5,25 @@
 
 name=
 
+topic=
+diff_opts=
+diff_committed_only=yes	# will be unset for index/worktree
+
 
 ## Parse options
 
 while [ -n "$1" ]; do
 	arg="$1"; shift
 	case "$arg" in
+	-i)
+		topic='(i)'
+		diff_opts="$diff_opts --cached";
+		diff_committed_only=;;
+	-w)
+		topic='(w)'
+		diff_committed_only=;;
 	-*)
-		echo "Usage: tg [...] patch [NAME]" >&2
+		echo "Usage: tg [...] patch [-i | -w] [NAME]" >&2
 		exit 1;;
 	*)
 		[ -z "$name" ] || die "name already specified ($name)"
@@ -20,31 +31,39 @@ while [ -n "$1" ]; do
 	esac
 done
 
+
+[ -n "$name"  -a  -z "$diff_committed_only" ]  &&
+	die "-i/-w are mutually exclusive with NAME"
+
 [ -n "$name" ] || name="$(git symbolic-ref HEAD | sed 's#^refs/\(heads\|top-bases\)/##')"
 base_rev="$(git rev-parse --short --verify "refs/top-bases/$name" 2>/dev/null)" ||
 	die "not a TopGit-controlled branch"
 
+# if not index/worktree, topic is current branch
+[ -z "$topic" ] && topic="$name"
+
+
 
 setup_pager
 
-git cat-file blob "$name:.topmsg"
+cat_file "$topic:.topmsg"
 echo
-[ -n "$(git grep '^[-]--' "$name" -- ".topmsg")" ] || echo '---'
+[ -n "$(git grep $diff_opts '^[-]--' ${diff_committed_only:+"$name"} -- ".topmsg")" ] || echo '---'
 
 # Evil obnoxious hack to work around the lack of git diff --exclude
 git_is_stupid="$(mktemp -t tg-patch-changes.XXXXXX)"
-git diff-tree --name-only "$base_rev" "$name" |
+git diff --name-only $diff_opts "$base_rev" ${diff_committed_only:+"$name"} -- |
 	fgrep -vx ".topdeps" |
 	fgrep -vx ".topmsg" >"$git_is_stupid" || : # fgrep likes to fail randomly?
 if [ -s "$git_is_stupid" ]; then
-	cat "$git_is_stupid" | xargs git diff --patch-with-stat "$base_rev" "$name" --
+	cat "$git_is_stupid" | xargs git diff --patch-with-stat $diff_opts "$base_rev" ${diff_committed_only:+"$name"} --
 else
 	echo "No changes."
 fi
 rm "$git_is_stupid"
 
 echo '-- '
-echo "tg: ($base_rev..) $name (depends on: $(git cat-file blob "$name:.topdeps" | paste -s -d' '))"
+echo "tg: ($base_rev..) $name (depends on: $(cat_file "$topic:.topdeps" | paste -s -d' '))"
 branch_contains "$name" "$base_rev" ||
 	echo "tg: The patch is out-of-date wrt. the base! Run \`$tg update\`."
 
diff --git a/tg.sh b/tg.sh
index b64fc3a..1762f03 100644
--- a/tg.sh
+++ b/tg.sh
@@ -17,6 +17,27 @@ die()
 	exit 1
 }
 
+# cat_file "topic:file"
+# Like `git cat-file blob $1`, but topics '(i)' and '(w)' means index and worktree
+cat_file()
+{
+	arg="$1"
+	case "$arg" in
+	'(w):'*)
+		arg=$(echo "$arg" | tail --bytes=+5)
+		cat "$arg"
+		return
+		;;
+	'(i):'*)
+		# ':file' means cat from index
+		arg=$(echo "$arg" | tail --bytes=+5)
+		git cat-file blob ":$arg"
+		;;
+	*)
+		git cat-file blob "$arg"
+	esac
+}
+
 # setup_hook NAME
 setup_hook()
 {
-- 
tg: (a3a5be1..) t/tg-patch-worktree (depends on: t/tg-patch-setup-pager)

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

On Fri, Jan 09, 2009 at 12:11:49AM +0300, Kirill Smelkov wrote:
> I'm sorry, but I've found a mistake in my code for case:
> 
> diff --git a/tg-patch.sh b/tg-patch.sh
> index db1ad09..d701c54 100644
> --- a/tg-patch.sh
> +++ b/tg-patch.sh
> @@ -17,8 +17,8 @@ while [ -n "$1" ]; do
>         case "$arg" in
>         -i)
>                 topic='(i)'
> -               diff_opts="$diff_opts --cached";;
> -               diff_committed_only=;
> +               diff_opts="$diff_opts --cached";
> +               diff_committed_only=;;
>         -w)
>                 topic='(w)'
>                 diff_committed_only=;;
> 
> 
> So here is corrected patch:
> 
> 
> From: Kirill Smelkov <kirr@landau.phys.spbu.ru>
> To: Petr Baudis <pasky@suse.cz>
> Cc: martin f krafft <madduck@madduck.net>
> Cc: Git Mailing List <git@vger.kernel.org>
> Subject: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index
> 
> This implements `tg patch -i` and `tg patch -w` to see current patch as
> generated against not-yet-committed index and worktree.
> 
> 
> NOTE: unfortunately `git cat-file blob <file>` does not provide an option
> to cat file from worktree (only from an object or from index), so I had to
> unroll my own `cat file topic:file` with special support for '(i)' and
> '(w)' topics.
> 
> Signed-off-by: Kirill Smelkov <kirr@landau.phys.spbu.ru>

[...]

Petr, Martin,

What's the state of this patch?

I don't understand why this gets ignored. Maybe I do something wrong?
...


Thanks,
Kirill

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.19.0206 +1100]:
> I don't understand why this gets ignored. Maybe I do something wrong?

I am not ignoring it, I am just currently flooded and travelling.
I don't see any obvious problems with your patch, but I do not have
the time to test it right now. I will do my best to do another
release when I return from Tasmania, hopefully on the plane trip
home. Maybe Petr can get around to it before I do.

Cheers, and sorry,

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
if you find a spelling mistake in the above, you get to keep it.
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

On Tue, Jan 20, 2009 at 11:21:03AM +1100, martin f krafft wrote:
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.19.0206 +1100]:
> > I don't understand why this gets ignored. Maybe I do something wrong?
> 
> I am not ignoring it, I am just currently flooded and travelling.
> I don't see any obvious problems with your patch, but I do not have
> the time to test it right now. I will do my best to do another
> release when I return from Tasmania, hopefully on the plane trip
> home. Maybe Petr can get around to it before I do.
> 
> Cheers, and sorry,

No problem and take your time.

It was just me having no idea about what's going on.


Happy travelling,
Kirill

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 749 bytes --]

also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.21.0456 +1100]:
> No problem and take your time.
> 
> It was just me having no idea about what's going on.

I am a little confused about the patches, and git-am does not seem
to be able to apply two patches from the same email. I use Maildir,
so I am passing the single message with multiple patches to git-am,
and it applies all changes into one single commit.

Anyway, do you have a repo which I can cherry-pick from?

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"out of the crooked timber of humanity,
 no straight thing was ever made."
                                                       -- imanuel kant
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[Kirill Smelkov]

On Wed, Jan 21, 2009 at 02:20:52PM +1100, martin f krafft wrote:
> also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.21.0456 +1100]:
> > No problem and take your time.
> > 
> > It was just me having no idea about what's going on.
> 
> I am a little confused about the patches, and git-am does not seem
> to be able to apply two patches from the same email. I use Maildir,
> so I am passing the single message with multiple patches to git-am,
> and it applies all changes into one single commit.

I see.

It seems I need to study how to prepare patches with patchy comments, or
get my hands dirty with `git am` :)

> Anyway, do you have a repo which I can cherry-pick from?

Sure

    git://repo.or.cz/topgit/kirr.git  for-upstream


Thanks,
Kirill

Re: [PATCH (topgit)] tg-patch: add support for generating patches against worktree and index

[martin f krafft]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

also sprach Kirill Smelkov <kirr@landau.phys.spbu.ru> [2009.01.22.0726 +1100]:
> It seems I need to study how to prepare patches with patchy
> comments, or get my hands dirty with `git am` :)

I believe git-send-email --editor can be used to create a lead-in
mail to describe what follows, and send patches in separate messages
in reply to that lead-in.

-- 
martin | http://madduck.net/ | http://two.sentenc.es/
 
"welcome to american airlines, sir. here's your avocado - remember to
 keep it turned on and with you at all times. please turn your luggage
 over to the armadillos for rootling."
                      -- http://azure.humbug.org.au/~aj/armadillos.txt
 
spamtraps: madduck.bogus@madduck.net

[-- Attachment #2: Digital signature (see http://martin-krafft.net/gpg/) --]
[-- Type: application/pgp-signature, Size: 197 bytes --]