* [PATCH 0/2] log/ format-patch improvements
@ 2010-08-21 20:28 Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits Ramkumar Ramachandra
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-21 20:28 UTC (permalink / raw)
To: Git Mailing List; +Cc: Thomas Rast, Jakub Narebski
Hi,
The first patch implements Jakub's suggestion. Arguably, it's slightly
complicated- it took me more than a few minutes to do the math with
`nr` and `nr_i`.
The second patch clarifies the meaning of the `-<n>` option. We should
also probably force the mutual exclusivity of `-<n>` and <revision
range> to avoid confusion.
Additionally, thanks to Thomas for drilling into me the fundamental
difference between -<n> and a revision range (on IRC).
Ramkumar Ramachandra (2):
git-format-patch: Print a diagnostic message when ignoring commits
log: Improve description of '-<n>' option in documentation
Documentation/git-format-patch.txt | 2 +-
Documentation/git-log.txt | 2 +-
builtin/log.c | 42 ++++++++++++++++++++++++++---------
3 files changed, 33 insertions(+), 13 deletions(-)
--
1.7.2.2.409.gdbb11.dirty
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits
2010-08-21 20:28 [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
@ 2010-08-21 20:28 ` Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 2/2] log: Improve description of '-<n>' option in documentation Ramkumar Ramachandra
2010-08-25 8:44 ` [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2 siblings, 0 replies; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-21 20:28 UTC (permalink / raw)
To: Git Mailing List; +Cc: Thomas Rast, Jakub Narebski
Earlier, git-format-patch used to silently skip over commits that it
didn't intend to make patches out of. As a consequence, a command like
'git-format-patch -3' would just do nothing and print nothing if the
topmost three commits were merge commits. Instead, print a useful
message similar to "Skipping: Merge branch ..." when ignoring a
commit.
Suggested-by: Jakub Narebski <jnareb@gmail.com>
Cc: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
builtin/log.c | 42 +++++++++++++++++++++++++++++++-----------
1 files changed, 31 insertions(+), 11 deletions(-)
diff --git a/builtin/log.c b/builtin/log.c
index 0151d2f..b64de7c 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1090,7 +1090,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
struct commit **list = NULL;
struct rev_info rev;
struct setup_revision_opt s_r_opt;
- int nr = 0, total, i;
+ int nr = 0, nr_i = 0, total, i;
int use_stdout = 0;
int start_number = -1;
int numbered_files = 0; /* _just_ numbers */
@@ -1098,6 +1098,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
int cover_letter = 0;
int boundary_count = 0;
int no_binary_diff = 0;
+ int *list_i = NULL;
struct commit *origin = NULL, *head = NULL;
const char *in_reply_to = NULL;
struct patch_ids ids;
@@ -1342,19 +1343,22 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
continue;
}
- /* ignore merges */
- if (commit->parents && commit->parents->next)
- continue;
-
- if (ignore_if_in_upstream &&
- has_commit_patch_id(commit, &ids))
- continue;
+ /* ignore merge commits and optionally ignore commits
+ already in upstream */
+ if ((commit->parents && commit->parents->next) ||
+ (ignore_if_in_upstream &&
+ has_commit_patch_id(commit, &ids))) {
+ /* Store the nr of the ignored commits in list_i */
+ nr_i++;
+ list_i = xrealloc(list_i, nr_i * sizeof(list_i[0]));
+ list_i[nr_i - 1] = nr;
+ }
nr++;
list = xrealloc(list, nr * sizeof(list[0]));
list[nr - 1] = commit;
}
- total = nr;
+ total = nr - nr_i;
if (!keep_subject && auto_number && total > 1)
numbered = 1;
if (numbered)
@@ -1376,10 +1380,25 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
start_number--;
}
rev.add_signoff = add_signoff;
- while (0 <= --nr) {
+ for (i = nr - nr_i; --nr >= 0;) {
int shown;
commit = list[nr];
- rev.nr = total - nr + (start_number - 1);
+
+ /* Ignore commits in list whose index is list_i */
+ if (list_i[nr_i - 1] == nr) {
+ struct strbuf commit_msg = STRBUF_INIT;
+ struct pretty_print_context ctx = {0};
+ format_commit_message(commit, "%s", &commit_msg, &ctx);
+ fprintf(realstdout, "Skipping: %s\n",
+ commit_msg.buf);
+ strbuf_release(&buf);
+ --nr_i;
+ continue;
+ }
+ else
+ --i;
+
+ rev.nr = total - i + (start_number - 1);
/* Make the second and subsequent mails replies to the first */
if (thread) {
/* Have we already had a message ID? */
@@ -1443,6 +1462,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
fclose(stdout);
}
free(list);
+ free(list_i);
string_list_clear(&extra_to, 0);
string_list_clear(&extra_cc, 0);
string_list_clear(&extra_hdr, 0);
--
1.7.2.2.409.gdbb11.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/2] log: Improve description of '-<n>' option in documentation
2010-08-21 20:28 [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits Ramkumar Ramachandra
@ 2010-08-21 20:28 ` Ramkumar Ramachandra
2010-08-25 8:44 ` [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2 siblings, 0 replies; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-21 20:28 UTC (permalink / raw)
To: Git Mailing List; +Cc: Thomas Rast, Jakub Narebski
The earlier description of the '-<n>' option was misleading- the user
would have expected to be able to use it to limit the number of
commits shown when specifying a revision range, for example. In
reality, the option simply instructs the log to walk the topmost <n>
commits. Also update the meaning of the same option in the
git-format-patch documentation.
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
Documentation/git-format-patch.txt | 2 +-
Documentation/git-log.txt | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 4b3f5ba..df77474 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -74,7 +74,7 @@ OPTIONS
include::diff-options.txt[]
-<n>::
- Limits the number of patches to prepare.
+ Prepare patches from the topmost <n> commits.
-o <dir>::
--output-directory <dir>::
diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 83e4ee3..ca02206 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -28,7 +28,7 @@ OPTIONS
-------
-<n>::
- Limits the number of commits to show.
+ Show the topmost <n> commits.
<since>..<until>::
Show only commits between the named two commits. When
--
1.7.2.2.409.gdbb11.dirty
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-21 20:28 [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 2/2] log: Improve description of '-<n>' option in documentation Ramkumar Ramachandra
@ 2010-08-25 8:44 ` Ramkumar Ramachandra
2010-08-25 20:54 ` Jonathan Nieder
2010-08-25 22:09 ` Junio C Hamano
2 siblings, 2 replies; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-25 8:44 UTC (permalink / raw)
To: Git Mailing List
Cc: Thomas Rast, Jakub Narebski, Junio C Hamano, Jonathan Nieder
Hi,
Ramkumar Ramachandra writes:
> The first patch implements Jakub's suggestion. Arguably, it's slightly
> complicated- it took me more than a few minutes to do the math with
> `nr` and `nr_i`.
>
> The second patch clarifies the meaning of the `-<n>` option. We should
> also probably force the mutual exclusivity of `-<n>` and <revision
> range> to avoid confusion.
>
> Additionally, thanks to Thomas for drilling into me the fundamental
> difference between -<n> and a revision range (on IRC).
>
> Ramkumar Ramachandra (2):
> git-format-patch: Print a diagnostic message when ignoring commits
> log: Improve description of '-<n>' option in documentation
>
> Documentation/git-format-patch.txt | 2 +-
> Documentation/git-log.txt | 2 +-
> builtin/log.c | 42 ++++++++++++++++++++++++++---------
> 3 files changed, 33 insertions(+), 13 deletions(-)
Do you see value in this patch or is it just unnecessary baggage?
-- Ram
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-25 8:44 ` [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
@ 2010-08-25 20:54 ` Jonathan Nieder
2010-08-26 5:34 ` Ramkumar Ramachandra
2010-08-25 22:09 ` Junio C Hamano
1 sibling, 1 reply; 11+ messages in thread
From: Jonathan Nieder @ 2010-08-25 20:54 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git Mailing List, Thomas Rast, Jakub Narebski, Junio C Hamano
Ramkumar Ramachandra wrote:
> Ramkumar Ramachandra writes:
>> The second patch clarifies the meaning of the `-<n>` option. We should
>> also probably force the mutual exclusivity of `-<n>` and <revision
>> range> to avoid confusion.
[...]
> Do you see value in this patch or is it just unnecessary baggage?
I see value in avoiding confusion. Maybe one solution would be to make
format-patch use --no-merges by default.
$ git log --oneline --no-merges -3 ab/test..origin/pu
70256a3 shell: Rewrite documentation and improve error message
9c46c05 rev-parse: tests git rev-parse --verify master@{n}, for various n
eedce78 sha1_name.c: use warning in preference to fprintf(stderr
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-25 8:44 ` [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2010-08-25 20:54 ` Jonathan Nieder
@ 2010-08-25 22:09 ` Junio C Hamano
1 sibling, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2010-08-25 22:09 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Git Mailing List, Thomas Rast, Jakub Narebski, Junio C Hamano,
Jonathan Nieder
Ramkumar Ramachandra <artagnon@gmail.com> writes:
> Ramkumar Ramachandra writes:
>> The first patch implements Jakub's suggestion. Arguably, it's slightly
>> complicated- it took me more than a few minutes to do the math with
>> `nr` and `nr_i`.
>>
>> The second patch clarifies the meaning of the `-<n>` option. We should
>> also probably force the mutual exclusivity of `-<n>` and <revision
>> range> to avoid confusion.
>>
>> Additionally, thanks to Thomas for drilling into me the fundamental
>> difference between -<n> and a revision range (on IRC).
>>
>> Ramkumar Ramachandra (2):
>> git-format-patch: Print a diagnostic message when ignoring commits
>> log: Improve description of '-<n>' option in documentation
>>
>> Documentation/git-format-patch.txt | 2 +-
>> Documentation/git-log.txt | 2 +-
>> builtin/log.c | 42 ++++++++++++++++++++++++++---------
>> 3 files changed, 33 insertions(+), 13 deletions(-)
>
> Do you see value in this patch or is it just unnecessary baggage?
I am not very impressed by the counting. It probably makes more sense to
count only what we are actually going to process and emit, i.e. always use
no-merges (do we even support format-patch on a merge?).
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-25 20:54 ` Jonathan Nieder
@ 2010-08-26 5:34 ` Ramkumar Ramachandra
2010-08-26 5:46 ` Jonathan Nieder
0 siblings, 1 reply; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-26 5:34 UTC (permalink / raw)
To: Jonathan Nieder, Junio C Hamano
Cc: Git Mailing List, Thomas Rast, Jakub Narebski
Hi Jonathan and Junio,
Junio C Hamano writes:
> I am not very impressed by the counting. It probably makes more sense to
> count only what we are actually going to process and emit, i.e. always use
> no-merges (do we even support format-patch on a merge?).
Frankly, I think the patch looks like an ugly hack myself. No,
format-patch doesn't support merge commits at all.
Jonathan Nieder writes:
> Ramkumar Ramachandra wrote:
> > Ramkumar Ramachandra writes:
>
> >> The second patch clarifies the meaning of the `-<n>` option. We should
> >> also probably force the mutual exclusivity of `-<n>` and <revision
> >> range> to avoid confusion.
> [...]
> > Do you see value in this patch or is it just unnecessary baggage?
>
> I see value in avoiding confusion. Maybe one solution would be to make
> format-patch use --no-merges by default.
Good idea. I'll write a patch. Do we also want people to be able to
turn off `--no-merges`? If so, how?
-- Ram
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-26 5:34 ` Ramkumar Ramachandra
@ 2010-08-26 5:46 ` Jonathan Nieder
2010-08-26 7:06 ` Thomas Rast
2010-08-26 15:37 ` Junio C Hamano
0 siblings, 2 replies; 11+ messages in thread
From: Jonathan Nieder @ 2010-08-26 5:46 UTC (permalink / raw)
To: Ramkumar Ramachandra
Cc: Junio C Hamano, Git Mailing List, Thomas Rast, Jakub Narebski,
Matthieu Moy
Ramkumar Ramachandra wrote:
> No,
> format-patch doesn't support merge commits at all.
[...]
> Jonathan Nieder writes:
>
>> I see value in avoiding confusion. Maybe one solution would be to make
>> format-patch use --no-merges by default.
>
> Good idea. I'll write a patch. Do we also want people to be able to
> turn off `--no-merges`?
I don't see a need for it.
However, if you can think of good names for --undo-no-merges and
--undo-merges options to "git log", that might be a nice independent
change for the revision option parser.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-26 5:46 ` Jonathan Nieder
@ 2010-08-26 7:06 ` Thomas Rast
2010-08-26 15:37 ` Junio C Hamano
1 sibling, 0 replies; 11+ messages in thread
From: Thomas Rast @ 2010-08-26 7:06 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ramkumar Ramachandra, Junio C Hamano, Git Mailing List,
Jakub Narebski, Matthieu Moy
Jonathan Nieder wrote:
> However, if you can think of good names for --undo-no-merges and
> --undo-merges options to "git log", that might be a nice independent
> change for the revision option parser.
--merges={include,only,exclude} or so, and then have --merges be
--merges=only and --no-merges be --merges=exclude?
--
Thomas Rast
trast@{inf,student}.ethz.ch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-26 5:46 ` Jonathan Nieder
2010-08-26 7:06 ` Thomas Rast
@ 2010-08-26 15:37 ` Junio C Hamano
2010-08-26 17:52 ` Ramkumar Ramachandra
1 sibling, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2010-08-26 15:37 UTC (permalink / raw)
To: Jonathan Nieder
Cc: Ramkumar Ramachandra, Git Mailing List, Thomas Rast,
Jakub Narebski, Matthieu Moy
Jonathan Nieder <jrnieder@gmail.com> writes:
> Ramkumar Ramachandra wrote:
> ...
>> Good idea. I'll write a patch. Do we also want people to be able to
>> turn off `--no-merges`?
>
> I don't see a need for it.
>
> However, if you can think of good names for --undo-no-merges and
> --undo-merges options to "git log", that might be a nice independent
> change for the revision option parser.
Wait a bit. How would you represent a merge in a patch form that can be
read by "git apply" (and "patch") in the first place?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] log/ format-patch improvements
2010-08-26 15:37 ` Junio C Hamano
@ 2010-08-26 17:52 ` Ramkumar Ramachandra
0 siblings, 0 replies; 11+ messages in thread
From: Ramkumar Ramachandra @ 2010-08-26 17:52 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jonathan Nieder, Git Mailing List, Thomas Rast, Jakub Narebski,
Matthieu Moy
Hi Junio,
Junio C Hamano writes:
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
> > Ramkumar Ramachandra wrote:
> > ...
> >> Good idea. I'll write a patch. Do we also want people to be able to
> >> turn off `--no-merges`?
> >
> > I don't see a need for it.
> >
> > However, if you can think of good names for --undo-no-merges and
> > --undo-merges options to "git log", that might be a nice independent
> > change for the revision option parser.
>
> Wait a bit. How would you represent a merge in a patch form that can be
> read by "git apply" (and "patch") in the first place?
Oh, I'm not attempting that. I merely wanted people to be able to turn
off `--no-merges` so they can get the current functionality.
As far as represeting merge commits go, I'm still thinking about it. I
talked to Thomas about it briefly on IRC. The main issues:
1. What's the point? When there's no conflict resolution, a merge
commit would be empty.
2. How do we uniquely specify what to merge? We can't use branch names
or commit SHA1s, because they can change.
-- Ram
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-08-26 17:54 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-08-21 20:28 [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 1/2] git-format-patch: Print a diagnostic message when ignoring commits Ramkumar Ramachandra
2010-08-21 20:28 ` [PATCH 2/2] log: Improve description of '-<n>' option in documentation Ramkumar Ramachandra
2010-08-25 8:44 ` [PATCH 0/2] log/ format-patch improvements Ramkumar Ramachandra
2010-08-25 20:54 ` Jonathan Nieder
2010-08-26 5:34 ` Ramkumar Ramachandra
2010-08-26 5:46 ` Jonathan Nieder
2010-08-26 7:06 ` Thomas Rast
2010-08-26 15:37 ` Junio C Hamano
2010-08-26 17:52 ` Ramkumar Ramachandra
2010-08-25 22:09 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).