threads / patch / 4082

patchfmt-patch: understand old <his> notation

Subject: [PATCH] fmt-patch: understand old <his> notation

## tl;dr

7 messages between May 6, 2006 and May 7, 2006. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Johannes Schindelin· May 6, 2006, 20:56 UTC · lore

When calling "git fmt-patch HEAD~5", you now get the same as if you would have said "git fmt-patch HEAD~5..". This makes it easier for my fingers which are so used to the old syntax.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	I wonder: would it make sense to make add_pending_object() and 
	get_reference() in revision.c non-static?

 builtin-diff.c |    2 +-
 builtin-log.c  |    8 ++++++++
 2 files changed, 9 insertions(+), 1 deletions(-)

Show changes to 2 files +9 −1

builtin-diff.c, builtin-log.c

diff --git a/builtin-diff.c b/builtin-diff.c
index 636edbf..2087316 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -232,7 +232,7 @@ static int builtin_diff_combined(struct 
 	return 0;
 }
 
-static void add_head(struct rev_info *revs)
+void add_head(struct rev_info *revs)
 {
 	unsigned char sha1[20];
 	struct object *obj;
diff --git a/builtin-log.c b/builtin-log.c
index 0027998..d5bbc1c 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -11,6 +11,9 @@ #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
 
+/* this is in builtin-diff.c */
+void add_head(struct rev_info *revs);
+
 static int cmd_log_wc(int argc, const char **argv, char **envp,
 		      struct rev_info *rev)
 {
@@ -185,6 +188,11 @@ int cmd_format_patch(int argc, const cha
 	if (argc > 1)
 		die ("unrecognized argument: %s", argv[1]);
 
+	if (rev.pending_objects && rev.pending_objects->next == NULL) {
+		rev.pending_objects->item->flags |= UNINTERESTING;
+		add_head(&rev);
+	}
+
 	if (!use_stdout)
 		realstdout = fdopen(dup(1), "w");

-- 
1.3.1.g9ba6-dirty

Junio C Hamano· May 6, 2006, 21:41 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fmt-patch: understand old <his> notation

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> When calling "git fmt-patch HEAD~5", you now get the same as if you would
> have said "git fmt-patch HEAD~5..". This makes it easier for my fingers
> which are so used to the old syntax.

While this would be easier on _my_ fingers as well, I have a suspicion that it might make more sense to make this "single ref" case to mean "HEAD~5^..HEAD~5" (if we _were_ designing a new command that is called format-patch today, that would be more natural). But probably it is too late to break it by now.

> 	I wonder: would it make sense to make add_pending_object() and 
> 	get_reference() in revision.c non-static?

I'd rather not expose such revision.c internals too much. An alternative approach would be to give instruction to revision.c (read: another flag like rev.no_walk) to tell it to do something special when the user has only one commit, but I think what you did in your patch is cleaner and sufficient.

Also we probably would want to default the diff options to show the root commit diff (rev.show_root_diff).

Johannes Schindelin· May 6, 2006, 22:01 UTC · re: Junio C Hamano · lore

Re: [PATCH] fmt-patch: understand old <his> notation

Hi,

On Sat, 6 May 2006, Junio C Hamano wrote:

Show 11 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > When calling "git fmt-patch HEAD~5", you now get the same as if you would
> > have said "git fmt-patch HEAD~5..". This makes it easier for my fingers
> > which are so used to the old syntax.
> 
> While this would be easier on _my_ fingers as well, I have a
> suspicion that it might make more sense to make this "single
> ref" case to mean "HEAD~5^..HEAD~5" (if we _were_ designing a
> new command that is called format-patch today, that would be
> more natural).  But probably it is too late to break it by now.

No, it is not too late. I did this patch only to prevent cluttering my directory with millions of patches, only because I forgot _again_ to type the two dots.

Show 8 quoted lines

> > 	I wonder: would it make sense to make add_pending_object() and 
> > 	get_reference() in revision.c non-static?
> 
> I'd rather not expose such revision.c internals too much.  An
> alternative approach would be to give instruction to revision.c
> (read: another flag like rev.no_walk) to tell it to do something
> special when the user has only one commit, but I think what you
> did in your patch is cleaner and sufficient.

I just stole the function add_head() from builtin-diff.c, but that feels wrong. I think adding a pending object should not be internal to revision.c.

> Also we probably would want to default the diff options to show
> the root commit diff (rev.show_root_diff).

I gather this is needed for git-am/git-rebase to continue working?

Ciao, Dscho

Junio C Hamano· May 7, 2006, 01:28 UTC · re: Johannes Schindelin · lore

Re: [PATCH] fmt-patch: understand old <his> notation

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> No, it is not too late. I did this patch only to prevent cluttering my 
> directory with millions of patches, only because I forgot _again_ to type 
> the two dots.

I did that as well, and the thing is, this is _so_ fast that when I noticed and typed ^C, it already has done 400 or so commits starting from the epoch (which was empty by the way).

I am already working on adjusting in-tree format-patch users not to use <his> syntax but to use <his>.. syntax, so this should not be a problem either way.

>> Also we probably would want to default the diff options to show
>> the root commit diff (rev.show_root_diff).

> I gather this is needed for git-am/git-rebase to continue working?

No, but people want to export the whole history sometimes.

Linus Torvalds· May 6, 2006, 22:30 UTC · re: Junio C Hamano · lore

Re: [PATCH] fmt-patch: understand old <his> notation

On Sat, 6 May 2006, Junio C Hamano wrote:

Show 6 quoted lines

> 
> While this would be easier on _my_ fingers as well, I have a
> suspicion that it might make more sense to make this "single
> ref" case to mean "HEAD~5^..HEAD~5" (if we _were_ designing a
> new command that is called format-patch today, that would be
> more natural).

Careful, that "X^..X" thing does entirely the wrong thing for merges. Not what you want at all, I think.

> But probably it is too late to break it by now.

Maybe not. I've actually cursed the fact that I made "git diff X" mean "diff from X to current working tree", because it almost never makes any sense at all when X is anything but "HEAD".

I probably _should_ have made "git diff X" mean basically "git show X", and not what it means now.

Oh, well.

			Linus

Olivier Galibert· May 6, 2006, 23:19 UTC · re: Linus Torvalds · lore

Re: [PATCH] fmt-patch: understand old <his> notation

On Sat, May 06, 2006 at 03:30:38PM -0700, Linus Torvalds wrote:

Show 6 quoted lines

> Maybe not. I've actually cursed the fact that I made "git diff X" mean 
> "diff from X to current working tree", because it almost never makes any 
> sense at all when X is anything but "HEAD".
> 
> I probably _should_ have made "git diff X" mean basically "git show X", 
> and not what it means now.

Funny, when tracking other projects I use (sometimes path-filtered) "git diff origin" often, but I find "git show origin" utterly uninteresting.

OG.

Junio C Hamano· May 7, 2006, 01:31 UTC · re: Linus Torvalds · lore

Re: [PATCH] fmt-patch: understand old <his> notation

Linus Torvalds <torvalds@osdl.org> writes:

> Maybe not. I've actually cursed the fact that I made "git diff X" mean 
> "diff from X to current working tree", because it almost never makes any 
> sense at all when X is anything but "HEAD".

Actually, I do "git diff next" all the time while on the tip of my topic branches, and also when merging a topic branch into "master". This "a different tree with the current working files" is probably the second most frequently used form for me (the first one is of course HEAD vs working files).