Re: Notes in format-patch
Junio C Hamano venit, vidit, dixit 13.11.2012 19:09: Junio C Hamano gits...@pobox.com writes: ... and it is broken X-. The blank line should be added before the diffstat, not after the notes message (t3307 shows a case where we give notes without diffstat, and we shouldn't be adding an extra blank line in that case. Second try. -- 8 -- Subject: format-patch: add a blank line between notes and diffstat The last line of the note text comes immediately before the diffstat block, making the latter unnecessarily harder to view. Signed-off-by: Junio C Hamano gits...@pobox.com --- log-tree.c | 31 +-- 1 file changed, 21 insertions(+), 10 deletions(-) Thanks, that patch works. I'm curious, though, where the empty line between the --- and your diffstat comes from. Do you have an empty note? I'm not getting any (origin/next+your patch). The fact that we don't usually have that empty line was the reason why I preferred to have no empty line between the --- and the Note:. Michael -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch
Michael J Gruber g...@drmicha.warpmail.net writes: Junio C Hamano venit, vidit, dixit 13.11.2012 19:09: Junio C Hamano gits...@pobox.com writes: ... and it is broken X-. The blank line should be added before the diffstat, not after the notes message (t3307 shows a case where we give notes without diffstat, and we shouldn't be adding an extra blank line in that case. Second try. -- 8 -- Subject: format-patch: add a blank line between notes and diffstat The last line of the note text comes immediately before the diffstat block, making the latter unnecessarily harder to view. Signed-off-by: Junio C Hamano gits...@pobox.com --- log-tree.c | 31 +-- 1 file changed, 21 insertions(+), 10 deletions(-) Thanks, that patch works. I'm curious, though, where the empty line between the --- and your diffstat comes from. The message you are responding to is *not* an output from format-patch but was written in my MUA. The way I work when I show this should work patch is to: (1) Think, edit in my working tree, compile, eyeball git diff HEAD, think again, and test; (2) Hit Reply All to the message I am going to give this should work response to, and start composing the response; (3) Run git diff --stat -p HEAD to have its output appended at the end of the message I started to compose in the previous step; (4) Write everything that should come before the output I appended in the previous step, i.e. -- 8 --, in-body Subject:, log, sign-off, and three-dash lines; (5) Send it out; and (6) Run git reset --hard and move on. The blank line was added in step (4), not step (3), which does not even have any commit log message, as the patch does not come from any existing commit. Later I may pick it up and apply to a topic branch just like I do for patches from other people and that is the point when such a patch becomes a commit for the first time. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Notes in format-patch (was: Re: [PATCHv3] replace: parse revision argument for -d)
Michael J Gruber venit, vidit, dixit 12.11.2012 15:18: 'git replace' parses the revision arguments when it creates replacements (so that a sha1 can be abbreviated, e.g.) but not when deleting replacements. Make it parse the argument to 'replace -d' in the same way. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- t/t6050-replace.sh | 11 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c By the way - Junio, is that the intented outcome of format-patch --notes? I would rather put the newline between the note and the diffstat (and omit the one after the ---) but may have goofed up a rebase: ... Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- ... -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch (was: Re: [PATCHv3] replace: parse revision argument for -d)
On Tue, Nov 13, 2012 at 11:30:19AM +0100, Michael J Gruber wrote: Michael J Gruber venit, vidit, dixit 12.11.2012 15:18: 'git replace' parses the revision arguments when it creates replacements (so that a sha1 can be abbreviated, e.g.) but not when deleting replacements. Make it parse the argument to 'replace -d' in the same way. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- t/t6050-replace.sh | 11 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c By the way - Junio, is that the intented outcome of format-patch --notes? I would rather put the newline between the note and the diffstat (and omit the one after the ---) but may have goofed up a rebase: I do not know was intended, but the above quoted output is hard to read, and your suggested change looks way better. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch
Michael J Gruber g...@drmicha.warpmail.net writes: Michael J Gruber venit, vidit, dixit 12.11.2012 15:18: 'git replace' parses the revision arguments when it creates replacements (so that a sha1 can be abbreviated, e.g.) but not when deleting replacements. Make it parse the argument to 'replace -d' in the same way. Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net --- Notes: v3 safeguards the hex buffer against reuse builtin/replace.c | 16 ++-- t/t6050-replace.sh | 11 +++ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/builtin/replace.c b/builtin/replace.c By the way - Junio, is that the intented outcome of format-patch --notes? I would rather put the newline between the note and the diffstat... I do not mind (actually I personally would prefer to see) a blank line between the three-dash and Notes:, but I agree that we should have a blank line before the diffstat block. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch
Junio C Hamano gits...@pobox.com writes:
Michael J Gruber g...@drmicha.warpmail.net writes:
Michael J Gruber venit, vidit, dixit 12.11.2012 15:18:
'git replace' parses the revision arguments when it creates replacements
(so that a sha1 can be abbreviated, e.g.) but not when deleting
replacements.
Make it parse the argument to 'replace -d' in the same way.
Signed-off-by: Michael J Gruber g...@drmicha.warpmail.net
---
Notes:
v3 safeguards the hex buffer against reuse
builtin/replace.c | 16 ++--
t/t6050-replace.sh | 11 +++
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/builtin/replace.c b/builtin/replace.c
By the way - Junio, is that the intented outcome of format-patch
--notes? I would rather put the newline between the note and the
diffstat...
I do not mind (actually I personally would prefer to see) a blank
line between the three-dash and Notes:, but I agree that we should
have a blank line before the diffstat block.
As the topic seems to be already in Peff's next, here is a trivial
fix for this in incremental form.
-- 8 --
Subject: format-patch: add a blank line between notes and diffstat
The last line of the note text comes immediately before the diffstat
block, making the latter unnecessarily harder to view.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
log-tree.c | 1 +
1 file changed, 1 insertion(+)
diff --git i/log-tree.c w/log-tree.c
index 712a22b..9303fd8 100644
--- i/log-tree.c
+++ w/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
opt-shown_dashes = 1;
}
strbuf_addstr(msgbuf, ctx.notes_message);
+ strbuf_addch(msgbuf, '\n');
}
if (opt-show_log_size) {
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch
Junio C Hamano gits...@pobox.com writes:
As the topic seems to be already in Peff's next, here is a trivial
fix for this in incremental form.
-- 8 --
Subject: format-patch: add a blank line between notes and diffstat
The last line of the note text comes immediately before the diffstat
block, making the latter unnecessarily harder to view.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
log-tree.c | 1 +
1 file changed, 1 insertion(+)
diff --git i/log-tree.c w/log-tree.c
index 712a22b..9303fd8 100644
--- i/log-tree.c
+++ w/log-tree.c
@@ -683,6 +683,7 @@ void show_log(struct rev_info *opt)
opt-shown_dashes = 1;
}
strbuf_addstr(msgbuf, ctx.notes_message);
+ strbuf_addch(msgbuf, '\n');
}
if (opt-show_log_size) {
... and it is broken X-.
The blank line should be added before the diffstat, not after the
notes message (t3307 shows a case where we give notes without
diffstat, and we shouldn't be adding an extra blank line in that
case.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Notes in format-patch
Junio C Hamano gits...@pobox.com writes:
... and it is broken X-.
The blank line should be added before the diffstat, not after the
notes message (t3307 shows a case where we give notes without
diffstat, and we shouldn't be adding an extra blank line in that
case.
Second try.
-- 8 --
Subject: format-patch: add a blank line between notes and diffstat
The last line of the note text comes immediately before the diffstat
block, making the latter unnecessarily harder to view.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
log-tree.c | 31 +--
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git c/log-tree.c w/log-tree.c
index 712a22b..4f86def 100644
--- c/log-tree.c
+++ w/log-tree.c
@@ -727,15 +727,16 @@ int log_tree_diff_flush(struct rev_info *opt)
}
if (opt-loginfo !opt-no_commit_id) {
- /* When showing a verbose header (i.e. log message),
-* and not in --pretty=oneline format, we would want
-* an extra newline between the end of log and the
-* output for readability.
-*/
show_log(opt);
if ((opt-diffopt.output_format ~DIFF_FORMAT_NO_OUTPUT)
opt-verbose_header
opt-commit_format != CMIT_FMT_ONELINE) {
+ /*
+* When showing a verbose header (i.e. log message),
+* and not in --pretty=oneline format, we would want
+* an extra newline between the end of log and the
+* diff/diffstat output for readability.
+*/
int pch = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_PATCH;
if (opt-diffopt.output_prefix) {
struct strbuf *msg = NULL;
@@ -743,11 +744,21 @@ int log_tree_diff_flush(struct rev_info *opt)
opt-diffopt.output_prefix_data);
fwrite(msg-buf, msg-len, 1, stdout);
}
- if (!opt-shown_dashes) {
- if ((pch opt-diffopt.output_format) == pch)
- printf(---);
- putchar('\n');
- }
+
+ /*
+* We may have shown three-dashes line early
+* between notes and the log message, in which
+* case we only want a blank line after the
+* notes without (an extra) three-dashes line.
+* Otherwise, we show the three-dashes line if
+* we are showing the patch with diffstat, but
+* in that case, there is no extra blank line
+* after the three-dashes line.
+*/
+ if (!opt-shown_dashes
+ (pch opt-diffopt.output_format) == pch)
+ printf(---);
+ putchar('\n');
}
}
diff_flush(opt-diffopt);
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
