Re: [PATCH v3] builtin/merge: support --squash --commit
On Thu, Jul 18, 2019 at 11:40 PM Edmundo Carmona Antoranz wrote: > > Using --squash made git stop regardless of conflicts so that the > user could finish the operation with a later call to git-commit. > > Now --squash --commit allows for the operation to finish with the > new revision if there are no conflicts. If the user does not use > --commit, then --no-commit is used as default so that it doesn't > break previous git behavior. > What should I do to get this patch to move forward? Either get comments (as the previous versions did... thanks, Junio!) or be accepted? Given that I didn't get a feedback I thought that it had made it (always the optimistic) but I see that it's not in Junio's 'what's cooking' mail from friday so I think it's fair to assume that this version of the patch is not gonna fly. Thanks in advance!
[PATCH v3] builtin/merge: support --squash --commit
Using --squash made git stop regardless of conflicts so that the
user could finish the operation with a later call to git-commit.
Now --squash --commit allows for the operation to finish with the
new revision if there are no conflicts. If the user does not use
--commit, then --no-commit is used as default so that it doesn't
break previous git behavior.
Function squash_message() now saves the value in merge_msg so that
the message with the squashed revisions is readily available when
calling finish_automerge() to create new revision object.
Function finish() used to skip execution paths if using --squash
because there would be no new revision object created. Also, it
now makes sure to skip reflog update if using --squash _without_
--commit.
Function finish_automerge() allows to create a new revision object
for an squashed merge (sets parent to current revision only,
merge_msg will be set to squashed revisions by calling
squash_message()), sets the reflog message to specify that it was
a merge-squash operation and removes the $GIT_DIR/SQUASH_MSG file
if needed.
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/merge.c | 93 +---
t/t7600-merge.sh | 86
2 files changed, 136 insertions(+), 43 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index aad5a9504c..ad9c6e900a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -390,11 +390,13 @@ static void finish_up_to_date(const char *msg)
static void squash_message(struct commit *commit, struct commit_list
*remoteheads)
{
struct rev_info rev;
- struct strbuf out = STRBUF_INIT;
struct commit_list *j;
struct pretty_print_context ctx = {0};
- printf(_("Squash commit -- not updating HEAD\n"));
+ strbuf_release(&merge_msg);
+
+ if (!option_commit)
+ printf(_("Squash commit -- not updating HEAD\n"));
repo_init_revisions(the_repository, &rev, NULL);
rev.ignore_merges = 1;
@@ -414,15 +416,14 @@ static void squash_message(struct commit *commit, struct
commit_list *remotehead
ctx.date_mode = rev.date_mode;
ctx.fmt = rev.commit_format;
- strbuf_addstr(&out, "Squashed commit of the following:\n");
+ strbuf_addstr(&merge_msg, "Squashed commit of the following:\n");
while ((commit = get_revision(&rev)) != NULL) {
- strbuf_addch(&out, '\n');
- strbuf_addf(&out, "commit %s\n",
+ strbuf_addch(&merge_msg, '\n');
+ strbuf_addf(&merge_msg, "commit %s\n",
oid_to_hex(&commit->object.oid));
- pretty_print_commit(&ctx, commit, &out);
+ pretty_print_commit(&ctx, commit, &merge_msg);
}
- write_file_buf(git_path_squash_msg(the_repository), out.buf, out.len);
- strbuf_release(&out);
+ write_file_buf(git_path_squash_msg(the_repository), merge_msg.buf,
merge_msg.len);
}
static void finish(struct commit *head_commit,
@@ -440,22 +441,22 @@ static void finish(struct commit *head_commit,
strbuf_addf(&reflog_message, "%s: %s",
getenv("GIT_REFLOG_ACTION"), msg);
}
- if (squash) {
+ if (squash)
squash_message(head_commit, remoteheads);
- } else {
- if (verbosity >= 0 && !merge_msg.len)
- printf(_("No merge message -- not updating HEAD\n"));
- else {
- const char *argv_gc_auto[] = { "gc", "--auto", NULL };
- update_ref(reflog_message.buf, "HEAD", new_head, head,
- 0, UPDATE_REFS_DIE_ON_ERR);
- /*
-* We ignore errors in 'gc --auto', since the
-* user should see them.
-*/
- close_object_store(the_repository->objects);
- run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
- }
+ if (verbosity >= 0 && !merge_msg.len)
+ printf(_("No merge message -- not updating HEAD\n"));
+ else if (squash && !option_commit)
+ ; /* avoid calling update_ref */
+ else {
+ const char *argv_gc_auto[] = { "gc", "--auto", NULL };
+ update_ref(reflog_message.buf, "HEAD", new_head, head,
+ 0, UPDATE_REFS_DIE_ON_ERR);
+ /*
+* We ignore errors in 'gc --auto', since the
+* user should see them.
+*/
+ close_object_store(the_repository->o
Re: [PATCH v2] builtin/merge: allow --squash to commit if there are no conflicts
On Wed, Jul 17, 2019 at 6:41 PM Edmundo Carmona Antoranz wrote: > > > Does it make sense to keep this file in those two situations? yes it does. disregard the question.
Re: [PATCH v2] builtin/merge: allow --squash to commit if there are no conflicts
On Wed, Jul 17, 2019 at 12:07 PM Junio C Hamano wrote: > > Sure. I started skimming and then gave up after seeing that quite a > lot of code has been shuffled around without much explanation (e.g. > printing of "Squash commit -- not updating HEAD" is gone from the > callee and now it is a responsibility of the caller), making it > harder than necessary to see if there is any unintended behaviour > change when the new feature is not in use. Whatever you are trying, > it does look like the change deserves to be split into a smaller > pieces to become more manageable. > > Thanks. > yw! I'm focusing on the squash --commit part only. I think I'm close to getting the desired result and now I'm taking a close look at the unit tests and a question came up on two tests of t7600-merge.sh: merge c0 with c1 (squash) merge c0 with c1 (squash, ff-only) In both cases it's a FF (right?) so no new revision is created. The unit tests are requiring that $GIT_DIR/squash_msg have some content: not ok 20 - merge c0 with c1 (squash, ff-only) # # git reset --hard c0 && # git merge --squash --ff-only c1 && # verify_merge file result.1 && # verify_head $c0 && # verify_no_mergehead && # test_cmp squash.1 .git/SQUASH_MSG not ok 18 - merge c0 with c1 (squash) # # git reset --hard c0 && # git merge --squash c1 && # verify_merge file result.1 && # verify_head $c0 && # verify_no_mergehead && # test_cmp squash.1 .git/SQUASH_MSG Does it make sense to keep this file in those two situations?
Re: [PATCH v2] builtin/merge: allow --squash to commit if there are no conflicts
On Fri, Jul 12, 2019 at 11:18 PM Edmundo Carmona Antoranz wrote: > > Option -m can be used to defined the message for the revision instead > of the default message that contains all squashed revisions info. > I have noticed that just adding the support for -m in squash is more complex than this patch is reaching so I think I will break this patch into two parts: - squash in a shot if there are no conflicts - support -m with squash Disregard this patch, please.
Re: [PATCH v2] builtin/merge: allow --squash to commit if there are no conflicts
On Fri, Jul 12, 2019 at 11:18 PM Edmundo Carmona Antoranz
wrote:
> @@ -1342,18 +1354,8 @@ int cmd_merge(int argc, const char **argv, const char
> *prefix)
> if (verbosity < 0)
> show_diffstat = 0;
>
> - if (squash) {
> - if (fast_forward == FF_NO)
> - die(_("You cannot combine --squash with --no-ff."));
> - if (option_commit > 0)
> - die(_("You cannot combine --squash with --commit."));
> - /*
> -* squash can now silently disable option_commit - this is not
> -* a problem as it is only overriding the default, not a user
> -* supplied option.
> -*/
> - option_commit = 0;
> - }
> + if (squash && fast_forward == FF_NO)
> + die(_("You cannot combine --squash with --no-ff."));
>
> if (option_commit < 0)
> option_commit = 1;
One question that I have is if it makes sense to set option_commit to
0 if the user didn't specify --commit when using --squash, so that the
current behavior of git is not broken. Like you run merge --squash,
git will stop as it currently does... but it would be possible to run
with --squash --commit so that the revision is created if there are no
issues to take care of (currently impossible, you would see that
message saying "You cannot combine --squash with --commit.").
[PATCH v2] builtin/merge: allow --squash to commit if there are no conflicts
Using --squash made git stop regardless of conflicts so that the
user could finish the operation with a later call to git-commit.
Now --squash allows for the operation to finish with the new revision
if there are no conflicts (can still be controlled with --no-commit).
Option -m can be used to defined the message for the revision instead
of the default message that contains all squashed revisions info.
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/merge.c | 109 +---
1 file changed, 57 insertions(+), 52 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index aad5a9504c..66fd57de02 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -64,6 +64,7 @@ static int option_edit = -1;
static int allow_trivial = 1, have_message, verify_signatures;
static int overwrite_ignore = 1;
static struct strbuf merge_msg = STRBUF_INIT;
+static struct strbuf squash_msg = STRBUF_INIT;
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
static const char **xopts;
@@ -390,39 +391,38 @@ static void finish_up_to_date(const char *msg)
static void squash_message(struct commit *commit, struct commit_list
*remoteheads)
{
struct rev_info rev;
- struct strbuf out = STRBUF_INIT;
struct commit_list *j;
struct pretty_print_context ctx = {0};
- printf(_("Squash commit -- not updating HEAD\n"));
-
- repo_init_revisions(the_repository, &rev, NULL);
- rev.ignore_merges = 1;
- rev.commit_format = CMIT_FMT_MEDIUM;
-
- commit->object.flags |= UNINTERESTING;
- add_pending_object(&rev, &commit->object, NULL);
-
- for (j = remoteheads; j; j = j->next)
- add_pending_object(&rev, &j->item->object, NULL);
-
- setup_revisions(0, NULL, &rev, NULL);
- if (prepare_revision_walk(&rev))
- die(_("revision walk setup failed"));
-
- ctx.abbrev = rev.abbrev;
- ctx.date_mode = rev.date_mode;
- ctx.fmt = rev.commit_format;
-
- strbuf_addstr(&out, "Squashed commit of the following:\n");
- while ((commit = get_revision(&rev)) != NULL) {
- strbuf_addch(&out, '\n');
- strbuf_addf(&out, "commit %s\n",
- oid_to_hex(&commit->object.oid));
- pretty_print_commit(&ctx, commit, &out);
+ if (merge_msg.len)
+ squash_msg = merge_msg;
+ else {
+ repo_init_revisions(the_repository, &rev, NULL);
+ rev.ignore_merges = 1;
+ rev.commit_format = CMIT_FMT_MEDIUM;
+
+ commit->object.flags |= UNINTERESTING;
+ add_pending_object(&rev, &commit->object, NULL);
+
+ for (j = remoteheads; j; j = j->next)
+ add_pending_object(&rev, &j->item->object, NULL);
+
+ setup_revisions(0, NULL, &rev, NULL);
+ if (prepare_revision_walk(&rev))
+ die(_("revision walk setup failed"));
+
+ ctx.abbrev = rev.abbrev;
+ ctx.date_mode = rev.date_mode;
+ ctx.fmt = rev.commit_format;
+
+ strbuf_addstr(&squash_msg, "Squashed commit of the
following:\n");
+ while ((commit = get_revision(&rev)) != NULL) {
+ strbuf_addch(&squash_msg, '\n');
+ strbuf_addf(&squash_msg, "commit %s\n",
+ oid_to_hex(&commit->object.oid));
+ pretty_print_commit(&ctx, commit, &squash_msg);
+ }
}
- write_file_buf(git_path_squash_msg(the_repository), out.buf, out.len);
- strbuf_release(&out);
}
static void finish(struct commit *head_commit,
@@ -440,8 +440,11 @@ static void finish(struct commit *head_commit,
strbuf_addf(&reflog_message, "%s: %s",
getenv("GIT_REFLOG_ACTION"), msg);
}
- if (squash) {
+ if (squash && !squash_msg.len) {
squash_message(head_commit, remoteheads);
+ write_file_buf(git_path_squash_msg(the_repository),
squash_msg.buf, squash_msg.len);
+ if (option_commit > 0)
+ printf(_("Squash conflicts -- not updating HEAD\n"));
} else {
if (verbosity >= 0 && !merge_msg.len)
printf(_("No merge message -- not updating HEAD\n"));
@@ -893,14 +896,23 @@ static int finish_automerge(struct commit *head,
struct object_id result_commit;
free_commit_list(common);
- parents = remoteheads;
- if (!head_subsumed || fast_forward == FF_NO)
-
[PATCH v2] builtin/merge.c - cleanup of code in for-cycle that tests strategies
The cmd_merge() function has a loop that tries different
merge strategies in turn, and stops when a strategy gets a
clean merge, while keeping the "best" conflicted merge so
far.
Make the loop easier to follow by moving the code around,
ensuring that there is only one "break" in the loop where
an automerge succeeds. Also group the actions that are
performed after an automerge succeeds together to a single
location, outside and after the loop.
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/merge.c | 53 +++--
1 file changed, 20 insertions(+), 33 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6e99aead46..e7aeedc77d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -892,6 +892,7 @@ static int finish_automerge(struct commit *head,
struct strbuf buf = STRBUF_INIT;
struct object_id result_commit;
+ write_tree_trivial(result_tree);
free_commit_list(common);
parents = remoteheads;
if (!head_subsumed || fast_forward == FF_NO)
@@ -1586,8 +1587,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
save_state(&stash))
oidclr(&stash);
- for (i = 0; i < use_strategies_nr; i++) {
- int ret;
+ for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
+ int ret, cnt;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
restore_state(&head_commit->object.oid, &stash);
@@ -1604,40 +1605,26 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
ret = try_merge_strategy(use_strategies[i]->name,
common, remoteheads,
head_commit);
- if (!option_commit && !ret) {
- merge_was_ok = 1;
- /*
-* This is necessary here just to avoid writing
-* the tree, but later we will *not* exit with
-* status code 1 because merge_was_ok is set.
-*/
- ret = 1;
- }
-
- if (ret) {
- /*
-* The backend exits with 1 when conflicts are
-* left to be resolved, with 2 when it does not
-* handle the given merge at all.
-*/
- if (ret == 1) {
- int cnt = evaluate_result();
-
- if (best_cnt <= 0 || cnt <= best_cnt) {
- best_strategy = use_strategies[i]->name;
- best_cnt = cnt;
+ /*
+* The backend exits with 1 when conflicts are
+* left to be resolved, with 2 when it does not
+* handle the given merge at all.
+*/
+ if (ret < 2) {
+ if (!ret) {
+ if (option_commit) {
+ /* Automerge succeeded. */
+ automerge_was_ok = 1;
+ break;
}
+ merge_was_ok = 1;
+ }
+ cnt = evaluate_result();
+ if (best_cnt <= 0 || cnt <= best_cnt) {
+ best_strategy = use_strategies[i]->name;
+ best_cnt = cnt;
}
- if (merge_was_ok)
- break;
- else
- continue;
}
-
- /* Automerge succeeded. */
- write_tree_trivial(&result_tree);
- automerge_was_ok = 1;
- break;
}
/*
--
2.20.1
Re: [PATCH v1] builtin/merge.c - cleanup of code in for-cycle that tests strategies
On Mon, Jul 8, 2019 at 2:15 PM Junio C Hamano wrote: > In the body of the proposed commit log message, please finish each > sentence with a full-stop. Ok > > These bullets are all subjective, and do not add any value to what > you already said in the second sentence. Ok > > These are facts that readers can see for themselves and agree with. > > Something like... > > The cmd_merge() function has a loop that tries different > merge strategies in turn, and stops when a strategy gets a > clean merge, while keeping the "best" conflicted merge so > far. > > Make the loop easier to follow by moving the code around, > ensuring that there is only one "break" in the loop where > an automerge succeeds. Also group the actions that are > performed after an automerge succeeds together to a single > location, outside and after the loop. > > perhaps? I like it. > > + int cnt = evaluate_result(); > This introduces -Wdeclaration-after-statement, doesn't it? It does! Let me take care of it. (Didn't know about DEVELOPER environment variable up until... 5 minutes ago?) > Perhaps just declare the variable at the top of the for loop, next > to where the local 'ret' is declared? Yep, make sense > > Other than this single glitch, I think the code with this patch does > become easier to follow. Great! Let me come back with v2 during the day. Thanks!
Re: [PATCH v1] builtin/merge.c - cleanup of code in for-cycle that tests strategies
On Sat, Jul 6, 2019 at 6:00 PM Edmundo Carmona Antoranz
wrote:
> @@ -1645,6 +1631,7 @@ int cmd_merge(int argc, const char **argv, const char
> *prefix)
> * auto resolved the merge cleanly.
> */
> if (automerge_was_ok) {
> + write_tree_trivial(&result_tree);
> ret = finish_automerge(head_commit, head_subsumed,
>common, remoteheads,
>&result_tree, wt_strategy);
I realized later that the call to write_tree_trivial could be included
in finish_automerge. Will include this change on v2 of the patch (if
there's a v2, depending on feedback).
[PATCH v1] builtin/merge.c - cleanup of code in for-cycle that tests strategies
Previous code was a little convoluted to follow logic
New code is shorter and logic is easier to follow
- Easier to see what happens when merge is successful
and how --no-commit affects result
- Simpler to see that for-cycle will stop when merge_was_ok is set
- Easier to spot what logic will run through best_strategy
- Easier to see that in case of ret being 2, cycle will continue
- Keep a single break case (when automerge succedes and a revision will
be created)
- Put together closing actions when automerge succedes if a revision
will be created
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/merge.c | 51 ++---
1 file changed, 19 insertions(+), 32 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6e99aead46..94f2713bea 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1586,7 +1586,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
save_state(&stash))
oidclr(&stash);
- for (i = 0; i < use_strategies_nr; i++) {
+ for (i = 0; !merge_was_ok && i < use_strategies_nr; i++) {
int ret;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
@@ -1604,40 +1604,26 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
ret = try_merge_strategy(use_strategies[i]->name,
common, remoteheads,
head_commit);
- if (!option_commit && !ret) {
- merge_was_ok = 1;
- /*
-* This is necessary here just to avoid writing
-* the tree, but later we will *not* exit with
-* status code 1 because merge_was_ok is set.
-*/
- ret = 1;
- }
-
- if (ret) {
- /*
-* The backend exits with 1 when conflicts are
-* left to be resolved, with 2 when it does not
-* handle the given merge at all.
-*/
- if (ret == 1) {
- int cnt = evaluate_result();
-
- if (best_cnt <= 0 || cnt <= best_cnt) {
- best_strategy = use_strategies[i]->name;
- best_cnt = cnt;
+ /*
+* The backend exits with 1 when conflicts are
+* left to be resolved, with 2 when it does not
+* handle the given merge at all.
+*/
+ if (ret < 2) {
+ if (!ret) {
+ if (option_commit) {
+ /* Automerge succeeded. */
+ automerge_was_ok = 1;
+ break;
}
+ merge_was_ok = 1;
+ }
+ int cnt = evaluate_result();
+ if (best_cnt <= 0 || cnt <= best_cnt) {
+ best_strategy = use_strategies[i]->name;
+ best_cnt = cnt;
}
- if (merge_was_ok)
- break;
- else
- continue;
}
-
- /* Automerge succeeded. */
- write_tree_trivial(&result_tree);
- automerge_was_ok = 1;
- break;
}
/*
@@ -1645,6 +1631,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
* auto resolved the merge cleanly.
*/
if (automerge_was_ok) {
+ write_tree_trivial(&result_tree);
ret = finish_automerge(head_commit, head_subsumed,
common, remoteheads,
&result_tree, wt_strategy);
--
2.20.1
[PATCH v1] merge - rename a shadowed variable in cmd_merge
variable ret used in cmd_merge introduced in d5a35c114ab was already
a local variable used inside a for loop inside the function.
for-local variable is being renamed to ret_try_merge to avoid shadow.
---
builtin/merge.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6e99aead46..972b6c376a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1587,7 +1587,7 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
oidclr(&stash);
for (i = 0; i < use_strategies_nr; i++) {
- int ret;
+ int ret_try_merge;
if (i) {
printf(_("Rewinding the tree to pristine...\n"));
restore_state(&head_commit->object.oid, &stash);
@@ -1601,26 +1601,26 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
*/
wt_strategy = use_strategies[i]->name;
- ret = try_merge_strategy(use_strategies[i]->name,
-common, remoteheads,
-head_commit);
- if (!option_commit && !ret) {
+ ret_try_merge = try_merge_strategy(use_strategies[i]->name,
+ common, remoteheads,
+ head_commit);
+ if (!option_commit && !ret_try_merge) {
merge_was_ok = 1;
/*
* This is necessary here just to avoid writing
* the tree, but later we will *not* exit with
* status code 1 because merge_was_ok is set.
*/
- ret = 1;
+ ret_try_merge = 1;
}
- if (ret) {
+ if (ret_try_merge) {
/*
* The backend exits with 1 when conflicts are
* left to be resolved, with 2 when it does not
* handle the given merge at all.
*/
- if (ret == 1) {
+ if (ret_try_merge == 1) {
int cnt = evaluate_result();
if (best_cnt <= 0 || cnt <= best_cnt) {
--
2.22.0.214.g8dca754b1e
Re: [RFC/PATCH v1] merge - make squash a 1-step operation
On Fri, Jul 5, 2019 at 10:56 AM Edmundo Carmona Antoranz wrote: > > --- > builtin/merge.c | 63 + > 1 file changed, 32 insertions(+), 31 deletions(-) > This would be a first step in order to achieve the dream of having rebase/squash in a single step. Let the flame-fest begin. I'm wondering if it makes sense to support -m with squash or if we should keep the default message with all the revisions that are being squashed (perhaps erroring out if the user mixes --squash and -m, just like mixing --squash and --no-ff) Thanks in advance for your feedback.
[RFC/PATCH v1] merge - make squash a 1-step operation
---
builtin/merge.c | 63 +
1 file changed, 32 insertions(+), 31 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 6e99aead46..d5745a7888 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -64,6 +64,7 @@ static int option_edit = -1;
static int allow_trivial = 1, have_message, verify_signatures;
static int overwrite_ignore = 1;
static struct strbuf merge_msg = STRBUF_INIT;
+static struct strbuf squash_msg = STRBUF_INIT;
static struct strategy **use_strategies;
static size_t use_strategies_nr, use_strategies_alloc;
static const char **xopts;
@@ -390,12 +391,9 @@ static void finish_up_to_date(const char *msg)
static void squash_message(struct commit *commit, struct commit_list
*remoteheads)
{
struct rev_info rev;
- struct strbuf out = STRBUF_INIT;
struct commit_list *j;
struct pretty_print_context ctx = {0};
- printf(_("Squash commit -- not updating HEAD\n"));
-
repo_init_revisions(the_repository, &rev, NULL);
rev.ignore_merges = 1;
rev.commit_format = CMIT_FMT_MEDIUM;
@@ -414,15 +412,13 @@ static void squash_message(struct commit *commit, struct
commit_list *remotehead
ctx.date_mode = rev.date_mode;
ctx.fmt = rev.commit_format;
- strbuf_addstr(&out, "Squashed commit of the following:\n");
+ strbuf_addstr(&squash_msg, "Squashed commit of the following:\n");
while ((commit = get_revision(&rev)) != NULL) {
- strbuf_addch(&out, '\n');
- strbuf_addf(&out, "commit %s\n",
+ strbuf_addch(&squash_msg, '\n');
+ strbuf_addf(&squash_msg, "commit %s\n",
oid_to_hex(&commit->object.oid));
- pretty_print_commit(&ctx, commit, &out);
+ pretty_print_commit(&ctx, commit, &squash_msg);
}
- write_file_buf(git_path_squash_msg(the_repository), out.buf, out.len);
- strbuf_release(&out);
}
static void finish(struct commit *head_commit,
@@ -440,8 +436,12 @@ static void finish(struct commit *head_commit,
strbuf_addf(&reflog_message, "%s: %s",
getenv("GIT_REFLOG_ACTION"), msg);
}
- if (squash) {
+ if (squash && !squash_msg.len) {
+ // message hasn't been calculated, that means we are stopping
the squash process so the user can finish it
squash_message(head_commit, remoteheads);
+ write_file_buf(git_path_squash_msg(the_repository),
squash_msg.buf, squash_msg.len);
+ if (option_commit > 0)
+ printf(_("Squash conflicts -- not updating HEAD\n"));
} else {
if (verbosity >= 0 && !merge_msg.len)
printf(_("No merge message -- not updating HEAD\n"));
@@ -893,13 +893,21 @@ static int finish_automerge(struct commit *head,
struct object_id result_commit;
free_commit_list(common);
- parents = remoteheads;
- if (!head_subsumed || fast_forward == FF_NO)
- commit_list_insert(head, &parents);
- prepare_to_commit(remoteheads);
- if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
- &result_commit, NULL, sign_commit))
- die(_("failed to write commit object"));
+ if (squash) {
+ squash_message(head, remoteheads);
+ parents = commit_list_insert(head, &parents);
+ if (commit_tree(squash_msg.buf, squash_msg.len, result_tree,
parents,
+ &result_commit, NULL, sign_commit))
+ die(_("failed to write commit object on squash"));
+ } else {
+ parents = remoteheads;
+ if (!head_subsumed || fast_forward == FF_NO)
+ commit_list_insert(head, &parents);
+ prepare_to_commit(remoteheads);
+ if (commit_tree(merge_msg.buf, merge_msg.len, result_tree,
parents,
+ &result_commit, NULL, sign_commit))
+ die(_("failed to write commit object"));
+ }
strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
finish(head, remoteheads, &result_commit, buf.buf);
strbuf_release(&buf);
@@ -1342,18 +1350,8 @@ int cmd_merge(int argc, const char **argv, const char
*prefix)
if (verbosity < 0)
show_diffstat = 0;
- if (squash) {
- if (fast_forward == FF_NO)
- die(_("You cannot combine --squash with --no-ff."));
- if (option_commit > 0)
- die(_("You cannot combine --squash with --commit."));
- /*
-* squash can now silently disable option_commit - this is not
-* a problem as it is only overriding the default, not a user
-* supplied option.
-*/
-
Re: [RFC/PATCH 1/2] rebuash - squash/rebase in a single step
On Mon, Jul 1, 2019 at 12:51 PM Junio C Hamano wrote:
>
>
> That part is understandable, but is "rebase-and-squash" a tool
> intended to be used by the contributor to respond to that request?
>
> Wouldn't the developer just do
>
> git checkout topic
> git fetch
> git rebase [-i] [@{upstream}]
> git push [publish +topic]
>
> to update the topic and ask to be pulled again? The two steps in
> the middle may be "pull --rebase", but my point is I do not quite
> see where the new squash/rebase-in-a-single-step thing comes into
> this picture. There may be a different picture that it fits, but
> I do not think it is this one.
>
>
I think rebasing -i makes sense* _if_ there are no conflicts on the
way to reach the current state of the branch if the developer was
pulling while developing the feature branch. If there are conflicts
that I took care of when i was pulling, i don't want to run rebase -i
to have to deal with them yet again. So rebuash would help developers
with or without merges with upstream on their feature branch, with or
without conflicts on those merges (if there are merges), get their
development into a single revision without having to write 4 or 5 git
commands, as Jeff was saying, if it makes workflows simpler
* rebase -i is the way I see people solve their "squash and rebase"
needs (pick the first revision, then squash all the others... but what
happens if the very first revision is conflicting with the current
state of upstream?), but I (for one) do it the way rebuash is doing:
merge with upstream, reset --soft to upstream, commit, voila! That's
why i wrote rebuash in the first place, just so you can see where I'm
coming from.
Re: [RFC/PATCH 1/2] rebuash - squash/rebase in a single step
On Sun, Jun 30, 2019 at 4:39 PM Jeff King wrote: > > > But perhaps the squashed version is easier to work with for further > modifications? I'm not sure how, though. Certainly in your example > rewriting changes in F1 with "rebase --interactive" would be a pain. But > I think the end-state of the tree after your rebuash is identical to > what you'd get by just merging from master. So in either case, just > building new work on top should be the same. > I'm still not quite sure of the greater workflow where having the > rebuash-ed commit on the feature branch is more useful than just having > a merge from master. Hmm... I as a gatekeeper would rather get either a straight line of revisions for a feature with no merges (even if a final merge takes care of solving conflicts with the upstream branch) or a single revision (if I thought that the change is not worth having more than a single revision). I'd ask the developer to rebase the whole thing and give a straight line (with rebase -i or cherry-picks) or to give me a single revision (where rebuash would come into the picture). Also, I wonder how it would make life easier for people that are learning to use git and the command that they see thrown around very often is to use `git pull` in order to get updates from the other developers. But that might be me being opinionated. PS About rebuash ordering not to use commit: Sure, at the moment, rebuash is not commit-safe or merge-continue-safe but I can add checks for that in case the user runs them before using rebuash --continue
Re: [RFC/PATCH 1/2] rebuash - squash/rebase in a single step
On Sun, Jun 30, 2019 at 12:54 AM Jeff King wrote:
>
>
> and then do:
>
> git merge --squash feature
>
> I get the same merge that rebuash is doing (with R6 as the merge base,
> so we see F5 and R7 conflicting with each other). And then when I finish
> it with "git commit", the result is a linear strand with M3 at the tip
> (and its commit message is even auto-populated with information from the
> squashed commits).
>
> -Peff
>From the point of view of the revisions that you produce in the end,
it's the same thing, but you are not rebasing/squashing your feature
branch, you are moving your upstream branch to where you want the
squashed/rebased branch to be. So, in fact you would need more steps,
something like (starting from your feature branch being checked out):
git checkout --detach $( git rev-parse --abbrev-ref --symbolic-full-name @{u} )
git merge --squash my-feature-branch
git branch -f my-feature-branch
git checkout my-feature-branch
Yes, it works. Only that with rebuash you would do (starting from the
feature branch being checked out branch):
git rebuash
as long as the upstream branch is set, of course.
I think it makes more sense in terms of development flow of feature
branches, if you know in the end you will give up a squashed branch:
modify
commit
modify
commit
git pull # no need to use pull --rebase, merges will be fine
modify
commit
modify
commit
git pull
git modify
# now I'm ready to rebase/squash
git fetch
git rebuash
adding history could be done with an additional option (--hist
(default) and --no-hist?)
But, as you said, it's not like it's not possible to do it (with a
little more effort) with available tools like merge --squash
Re: [RFC/PATCH 2/2] rebuash - support for status
On Sat, Jun 29, 2019 at 11:18 PM Edmundo Carmona Antoranz wrote: > > --- > wt-status.c | 49 +++-- > wt-status.h | 1 + > 2 files changed, 44 insertions(+), 6 deletions(-) > I bet there are more things to do... like: - what happens if the user runs git checkout --force some-branch? The state file has to be deleted, right? - what should happen on the reflog when there's the merge then reset? Is it possible to control what will show up on the reflog from the script? So any guiding word on the things I should take a look at would be greatly appreciated.
[RFC/PATCH 2/2] rebuash - support for status
---
wt-status.c | 49 +++--
wt-status.h | 1 +
2 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index 0bccef542f..2a7627b331 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1118,11 +1118,24 @@ static void show_merge_in_progress(struct wt_status *s,
if (has_unmerged(s)) {
status_printf_ln(s, color, _("You have unmerged paths."));
if (s->hints) {
- status_printf_ln(s, color,
-_(" (fix conflicts and run \"git
commit\")"));
- status_printf_ln(s, color,
-_(" (use \"git merge --abort\" to
abort the merge)"));
+ if (s->state.rebuash_in_progress) {
+ status_printf_ln(s, color,
+ _(" (fix conflicts and run
\"git rebuash --continue\")"));
+ status_printf_ln(s, color,
+ _(" (use \"git rebuash
--abort\" to abort the operation)"));
+ } else {
+ status_printf_ln(s, color,
+ _(" (fix conflicts and run
\"git commit\")"));
+ status_printf_ln(s, color,
+ _(" (use \"git merge --abort\"
to abort the merge)"));
+ }
}
+ } else if (s->state.rebuash_in_progress) {
+ status_printf_ln(s, color,
+ _("All conflicts fixed but you are still rebuashing."));
+ if (s->hints)
+ status_printf_ln(s, color,
+ _(" (use \"git rebuash --continue\" to
conclude rebuash)"));
} else {
status_printf_ln(s, color,
_("All conflicts fixed but you are still merging."));
@@ -1386,6 +1399,15 @@ static void show_rebase_in_progress(struct wt_status *s,
wt_longstatus_print_trailer(s);
}
+static void show_rebuash_in_progress(struct wt_status *s,
+ const char *color)
+{
+ if (&s->state.rebuash_in_progress)
+ status_printf_ln(s, color,
+ _("Rebuash currently in progress."));
+ wt_longstatus_print_trailer(s);
+}
+
static void show_cherry_pick_in_progress(struct wt_status *s,
const char *color)
{
@@ -1583,6 +1605,18 @@ int wt_status_check_rebase(const struct worktree *wt,
return 1;
}
+int wt_status_check_rebuash(const struct worktree *wt,
+ struct wt_status_state *state)
+{
+ struct stat st;
+
+ if (!stat(worktree_git_path(wt, "REBUASH_STATE"), &st)) {
+ state->rebuash_in_progress = 1;
+ } else
+ return 0;
+ return 1;
+}
+
int wt_status_check_bisect(const struct worktree *wt,
struct wt_status_state *state)
{
@@ -1614,6 +1648,7 @@ void wt_status_get_state(struct repository *r,
state->cherry_pick_in_progress = 1;
oidcpy(&state->cherry_pick_head_oid, &oid);
}
+ wt_status_check_rebuash(NULL, state);
wt_status_check_bisect(NULL, state);
if (!stat(git_path_revert_head(r), &st) &&
!get_oid("REVERT_HEAD", &oid)) {
@@ -1643,6 +1678,8 @@ static void wt_longstatus_print_state(struct wt_status *s)
show_rebase_information(s, state_color);
fputs("\n", s->fp);
}
+ if (state->rebuash_in_progress)
+ show_rebuash_in_progress(s, state_color);
show_merge_in_progress(s, state_color);
} else if (state->am_in_progress)
show_am_in_progress(s, state_color);
@@ -1688,7 +1725,7 @@ static void wt_longstatus_print(struct wt_status *s)
status_printf(s, color(WT_STATUS_HEADER, s), "%s", "");
status_printf_more(s, branch_status_color, "%s", on_what);
status_printf_more(s, branch_color, "%s\n", branch_name);
- if (!s->is_initial)
+ if (!(s->is_initial || s->state.rebuash_in_progress))
wt_longstatus_print_tracking(s);
}
@@ -1731,7 +1768,7 @@ static void wt_longstatus_print(struct wt_status *s)
if (s->verbose)
wt_longstatus_print_verbose(s);
- if (!s->committable) {
+ if (!(s->committable || s->state.rebuash_in_progress)) {
if (s->amend)
status_printf_ln(s, GIT_COLOR_NORMAL, _("No changes"));
else if (s->nowarn)
diff --git a/wt-status.h b/wt-status.h
index 64f1ddc9fd..2995866327 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -72,6 +72,7 @@ struct wt_st
[RFC/PATCH 1/2] rebuash - squash/rebase in a single step
plumbingmanipulators
git-rebase mainporcelain history
+git-rebuash mainporcelain history
git-receive-packsynchelpers
git-reflog ancillarymanipulators
complete
git-remote ancillarymanipulators
complete
diff --git a/git-rebuash.sh b/git-rebuash.sh
new file mode 100644
index 00..93c14cb9cc
--- /dev/null
+++ b/git-rebuash.sh
@@ -0,0 +1,324 @@
+#!/bin/bash
+# Copyright 2019 Edmundo Carmona Antoranz
+
+# Released under the terms of GPLv2
+
+# Rebuash will be used mainly when you want to squash and rebase
+# so that you don't have to go through the ordeal of rebasing
+# which would potencially bring conflicts that you would
+# rather avoid it at all possible.
+
+# rebuash takes a different strategy by merging with the target (upstream)
branch
+# then resetting --soft onto the target branch to finish off with a new
+# revision
+
+# Valid options:
+# -u upstream branch (if it's not specified, it will be retrieved from current
branch)
+# -m comment for the final revision (can be multiline)
+# --abort if the user wants to abort the rebuash operation
+# --continue if there was a previous conflict and the user wants to continue
with the current rebuash operation
+
+UPSTREAM="" # upstream branch (will be retrieved from current branch if it is
not provided)
+HEAD="" # the point where we started rebuash from (revision or branch, for
messaging)
+HEAD_REV="" # point where we started the operation from (the revision)
+STEP="" # will be used when we need to continue to know in which step we are
at the moment
+COMMENT="" # comment to be used on the final revision (might be empty)
+
+# actions
+START=true
+CONTINUE=false
+ABORT=false
+
+. git-sh-setup
+
+STATE_FILE="$GIT_DIR"/REBUASH_STATE
+
+function report_bug {
+echo "You just hit a bug in git-rebuash"
+echo "BUG: $1"
+echo "Please, report this problem to the git mailing list and cc eantoranz
at gmail.com"
+exit 1
+}
+
+function save_state {
+echo "UPSTREAM: $UPSTREAM" > "$STATE_FILE"
+echo "HEAD: $HEAD" >> "$STATE_FILE"
+echo "HEAD_REV: $HEAD_REV" >> "$STATE_FILE"
+echo "STEP: $STEP" >> "$STATE_FILE"
+echo "" >> "$STATE_FILE" # an empty line
+echo "$COMMENT" >> "$STATE_FILE"
+}
+
+function read_state {
+# read the content of state file and put it into the variables
+if [ ! -f "$STATE_FILE" ]
+then
+echo Unexpected Error: Rebuash state file was expected to exist.
+echo Aborting operation.
+echo Please, notify the git mail list and explain them what the
situation was
+echo so that this bug can be taken care of.
+exit 1
+fi
+UPSTREAM=$( head -n 1 "$STATE_FILE" | sed 's/UPSTREAM: //' )
+HEAD=$( head -n 2 "$STATE_FILE" | tail -n 1 | sed 's/HEAD: //' )
+HEAD_REV=$( head -n 3 "$STATE_FILE" | tail -n 1 | sed 's/HEAD_REV: //' )
+STEP=$( head -n 4 "$STATE_FILE" | tail -n 1 | sed 's/STEP: //' )
+# there is an empty line
+COMMENT=$( tail -n +6 "$STATE_FILE" )
+}
+
+function remove_state {
+if [ -f "$STATE_FILE" ]
+then
+rm -f "$STATE_FILE"
+fi
+}
+
+function check_status {
+if [ $CONTINUE == true ]
+then
+if [ $ABORT == true ]
+then
+echo Can\'t use --abort and --continue at the same time
+exit 1
+fi
+
+# there has to be an state file
+if [ ! -f "$STATE_FILE" ]
+then
+echo There\'s no rebuash session currently going on. Can\'t
continue.
+exit 1
+fi
+elif [ $ABORT == true ]
+then
+if [ ! -f "$STATE_FILE" ]
+then
+echo There\'s no rebuash session currently going on. Can\'t abort.
+exit 1
+fi
+else
+if [ $START != true ]
+then
+report_bug "START is set to false even though we were not aborting
or continuing"
+fi
+
+if [ "$UPSTREAM" == "" ]
+then
+# as a fallback, try to get upstream from current branch
+UPSTREAM=$( git rev-parse --abbrev-ref --symbolic-full-name @{u}
2> /dev/null )
+if [ "$UPSTREAM" == "" ]
+then
+echo "Could not find out upstream branch. Please provide it
with -u"
+exit 1
+else
+echo Using $UPSTREAM as the upstream branch
+fi
+fi
+
+
Re: [PATCH] bisect: add quit command
I found out about "git bisect reset HEAD" while working on "git bisect
quit" but I think it's still worth it.
Let me know.
On Sat, Mar 25, 2017 at 3:17 PM, Edmundo Carmona Antoranz
wrote:
> git bisect quit will call git reset HEAD so that the working tree
> remains at the current revision
> ---
> Documentation/git-bisect.txt | 12
> git-bisect.sh| 11 ++-
> 2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
> index bdd915a66..de79e9e44 100644
> --- a/Documentation/git-bisect.txt
> +++ b/Documentation/git-bisect.txt
> @@ -23,6 +23,7 @@ on the subcommand:
> git bisect terms [--term-good | --term-bad]
> git bisect skip [(|)...]
> git bisect reset []
> + git bisect quit
> git bisect visualize
> git bisect replay
> git bisect log
> @@ -120,6 +121,17 @@ bad revision, while `git bisect reset HEAD` will leave
> you on the
> current bisection commit and avoid switching commits at all.
>
>
> +Bisect quit
> +~~~
> +
> +It's an equivalent of bisect reset but that stays at the current
> +revision instead of taking your working tree to the starting revision.
> +
> +
> +$ git bisect quit
> +
> +
> +
> Alternate terms
> ~~~
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index ae3cb013e..adbff2c69 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -1,6 +1,6 @@
> #!/bin/sh
>
> -USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
> +USAGE='[help|start|bad|good|new|old|terms|skip|next|quit|reset|visualize|replay|log|run]'
> LONG_USAGE='git bisect help
> print this long help message.
> git bisect start [--term-{old,good}= --term-{new,bad}=]
> @@ -20,6 +20,9 @@ git bisect next
> find next bisection to test and check it out.
> git bisect reset []
> finish bisection search and go back to commit.
> +git bisect quit
> + stop bisection on its tracks and stay on current revision.
> + Equivalent to git bisect reset HEAD
> git bisect visualize
> show bisect status in gitk.
> git bisect replay
> @@ -433,6 +436,10 @@ Try 'git bisect reset '.")"
> bisect_clean_state
> }
>
> +bisect_quit() {
> + bisect_reset "HEAD"
> +}
> +
> bisect_clean_state() {
> # There may be some refs packed during bisection.
> git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* |
> @@ -683,6 +690,8 @@ case "$#" in
> bisect_visualize "$@" ;;
> reset)
> bisect_reset "$@" ;;
> + quit)
> + bisect_quit ;;
> replay)
> bisect_replay "$@" ;;
> log)
> --
> 2.11.0.rc1
>
[PATCH] bisect: add quit command
git bisect quit will call git reset HEAD so that the working tree
remains at the current revision
---
Documentation/git-bisect.txt | 12
git-bisect.sh| 11 ++-
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt
index bdd915a66..de79e9e44 100644
--- a/Documentation/git-bisect.txt
+++ b/Documentation/git-bisect.txt
@@ -23,6 +23,7 @@ on the subcommand:
git bisect terms [--term-good | --term-bad]
git bisect skip [(|)...]
git bisect reset []
+ git bisect quit
git bisect visualize
git bisect replay
git bisect log
@@ -120,6 +121,17 @@ bad revision, while `git bisect reset HEAD` will leave you
on the
current bisection commit and avoid switching commits at all.
+Bisect quit
+~~~
+
+It's an equivalent of bisect reset but that stays at the current
+revision instead of taking your working tree to the starting revision.
+
+
+$ git bisect quit
+
+
+
Alternate terms
~~~
diff --git a/git-bisect.sh b/git-bisect.sh
index ae3cb013e..adbff2c69 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,6 +1,6 @@
#!/bin/sh
-USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
+USAGE='[help|start|bad|good|new|old|terms|skip|next|quit|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
git bisect start [--term-{old,good}= --term-{new,bad}=]
@@ -20,6 +20,9 @@ git bisect next
find next bisection to test and check it out.
git bisect reset []
finish bisection search and go back to commit.
+git bisect quit
+ stop bisection on its tracks and stay on current revision.
+ Equivalent to git bisect reset HEAD
git bisect visualize
show bisect status in gitk.
git bisect replay
@@ -433,6 +436,10 @@ Try 'git bisect reset '.")"
bisect_clean_state
}
+bisect_quit() {
+ bisect_reset "HEAD"
+}
+
bisect_clean_state() {
# There may be some refs packed during bisection.
git for-each-ref --format='%(refname) %(objectname)' refs/bisect/\* |
@@ -683,6 +690,8 @@ case "$#" in
bisect_visualize "$@" ;;
reset)
bisect_reset "$@" ;;
+ quit)
+ bisect_quit ;;
replay)
bisect_replay "$@" ;;
log)
--
2.11.0.rc1
blame --line-porcelain is providing me with funny output
Hi, everybody! As part of my improvements to difflame I want to use revision information as provided by blame --line-porcelain so that I can avoid some git calls to cat-file -p to get revision information hoping that information would be a match. However I'm not finding that to be the case. $ git blame --no-progress -w --line-porcelain -L 72,72 4e59582ff70d299f5a88449891e78d15b4b3fabe -- t/lib-submodule-update.sh 3290fe6dd2a7e2bb35ac760443335dec58802ff1 72 72 1 author Stefan Beller author-mail author-time 1484160452 author-tz -0800 committer Junio C Hamano committer-mail committer-time 1484337771 committer-tz -0800 summary lib-submodule-update.sh: reduce use of subshell by using "git -C" previous d7dffce1cebde29a0c4b309a79e4345450bf352a t/lib-submodule-update.sh filename t/lib-submodule-update.sh git -C sub1 checkout modifications && If we then take a look at the information on that revision using cat-file -p on the revision: $ git cat-file -p 3290fe6dd2a7e2bb35ac760443335dec58802ff1 tree 7df89dad28ec8b08875395265a3f2e13ba180174 parent d7dffce1cebde29a0c4b309a79e4345450bf352a author Stefan Beller 1484160452 -0800 committer Junio C Hamano 1484337771 -0800 (which, just in case, resembles the information provided by git show... but is simpler to parse with cat-file). Committer mails are matching, however author mail does not match between line-porcelain and cat-file. Is there a reason for that? Thanks in advance.
Re: [PATCH] blame: draft of line format
On Tue, Jan 31, 2017 at 1:41 PM, Jeff King wrote:
> On Mon, Jan 30, 2017 at 08:28:30PM -0600, Edmundo Carmona Antoranz wrote:
>
>> +static void pretty_info(char* revid, struct blame_entry *ent, struct strbuf
>> *rev_buffer)
>> +{
>> + struct pretty_print_context ctx = {0};
>> + struct rev_info rev;
>> +
>> + struct strbuf format = STRBUF_INIT;
>> + strbuf_addstr(&format, format_line);
>> + ctx.fmt = CMIT_FMT_USERFORMAT;
>> + get_commit_format(format.buf, &rev);
>> + pretty_print_commit(&ctx, ent->suspect->commit, rev_buffer);
>> + strbuf_release(&format);
>> +}
>
> I think this may be less awkward if you use format_commit_message() as
> the entry point. Then you do not need a rev_info struct at all, it
> touches fewer global variables, etc.
>
> I don't know if that would cause the other difficulties you mentioned,
> though.
>
> -Peff
Thanks for the tip, Peff. It made the code to get rev info much
shorter. I'll work on some other improvements and then I'll send
another patch.
Best regards!
difflame improvements
Hi!
Since my last post the biggest improvement is the ability to detect
that the user has requested a "reverse" analysis.
Under "normal" circumstances a user would ask difflame to get the diff
from an ancestor (call "difflame treeish1 treeish2" so that merge-base
of treeish1 treeish2 equals treeish1). In this case the blame result
is done using straight blame output for added lines and additional
analysis to detect where a line was deleted (analysis has improved a
lot in this regard I haven't heard anything from Peff, though).
But if the user requests the opposite (call "difflame treeish1
treeish2" so that merge-base of treeish1 treeish2 is treeish2) then
the analysis has to be driven "in reverse".
Here's one example taken from difflame itself:
normal "forward" call (hope output doesn't get butchered):
$ ./difflame.py HEAD~3 HEAD~2
diff --git a/difflame.py b/difflame.py
index e70154a..04c7577 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
e5b218e4 (Edmundo 2017-02-01 365) # we already had the revision
50528377 (Edmundo 2017-03-04 366) return REVISIONS_ID_CACHE[revision]
d1d11d8a (Edmundo 2017-02-02 367) # fallback to get it from git
b1a6693 use rev-list to get revision IDs
-b1a6693 (Edmundo 2017-03-04 368) full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
+b1a66932 (Edmundo 2017-03-04 368) full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
50528377 (Edmundo 2017-03-04 369) REVISIONS_ID_CACHE[revision] =
full_revision
e5b218e4 (Edmundo 2017-02-01 370) return full_revision
91b7d3f5 (Edmundo 2017-01-31 371)
"reverse" call:
$ ./difflame.py HEAD~2 HEAD~3
diff --git a/difflame.py b/difflame.py
index 04c7577..e70154a 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
e5b218e4 (Edmundo 2017-02-01 365) # we already had the revision
50528377 (Edmundo 2017-03-04 366) return REVISIONS_ID_CACHE[revision]
d1d11d8a (Edmundo 2017-02-02 367) # fallback to get it from git
b1a6693 use rev-list to get revision IDs
-b1a66932 (Edmundo 2017-03-04 368) full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
+b1a6693 (Edmundo 2017-03-04 368) full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
50528377 (Edmundo 2017-03-04 369) REVISIONS_ID_CACHE[revision] =
full_revision
e5b218e4 (Edmundo 2017-02-01 370) return full_revision
91b7d3f5 (Edmundo 2017-01-31 371)
Notice how the revision reported in both difflame calls is the same:
$ git show b1a66932
commit b1a66932704245fd653f8d48c0a718f168f334a7
Author: Edmundo Carmona Antoranz
Date: Sat Mar 4 13:59:50 2017 -0600
use rev-list to get revision IDs
diff --git a/difflame.py b/difflame.py
index e70154a..04c7577 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
# we already had the revision
return REVISIONS_ID_CACHE[revision]
# fallback to get it from git
-full_revision = run_git_command(["show", "--pretty=%H",
revision]).split("\n")[0]
+full_revision = run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
REVISIONS_ID_CACHE[revision] = full_revision
return full_revision
If this "detection" to perform reverse analysis hadn't been done, then
there wouldn't be a lot of useful information because there are no
revisions in HEAD~2..HEAD~3 and so the output would have been
something like:
diff --git a/difflame.py b/difflame.py
index 04c7577..e70154a 100755
--- a/difflame.py
+++ b/difflame.py
@@ -365,7 +365,7 @@ def get_full_revision_id(revision):
e5b218e4 (Edmundo 2017-02-01 365) # we already had the revision
50528377 (Edmundo 2017-03-04 366) return REVISIONS_ID_CACHE[revision]
d1d11d8a (Edmundo 2017-02-02 367) # fallback to get it from git
b1a6693 use rev-list to get revision IDs
%b1a6693 (Edmundo 2017-03-04 368) full_revision =
run_git_command(["rev-list", "--max-count=1",
revision]).split("\n")[0]
e5b218e printing hints for deleted lines
+e5b218e4 (Edmundo 2017-02-01 368) full_revision =
run_git_command(["show", "--pretty=%H", revision]).split("\n")[0]
50528377 (Edmundo 2017-03-04 369) REVISIONS_ID_CACHE[revision] =
full_revision
e5b218e4 (Edmundo 2017-02-01 370) return full_revision
91b7d3f5 (Edmundo 2017-01-31 371)
Notice how both the added line and the deleted line are reporting the
_wrong_ revision. It should be b1a66932 in all cases.
One quest
Re: difflame improvements
On Fri, Feb 17, 2017 at 1:01 AM, Edmundo Carmona Antoranz wrote: > On Thu, Feb 16, 2017 at 11:17 PM, Jeff King wrote: > >> This isn't difflame's fault; that's what "git blame" tells you about >> that line. But since I already told difflame "v2.6.5..HEAD", it would >> probably make sense to similarly limit the blame to that range. That >> turns up a boundary commit for the line. Which is _also_ not helpful, >> but at least the tool is telling me that the line came from before >> v2.6.5, and I don't really need to care much about it. > > > I'm running my own tests on difflame and I have a theory about "when" > it breaks at least one of the cases when it breaks: > > Analysis for deleted lines is being driven by git blame --reverse. > What I have noticed is that it "breaks" when blame --reverse drives > the analysis into revisions where "treeish1" is not part of their > history (like, bringing analysis "to the sides" of treeish1 instead of > keeping analysis in revisions in the history of treeish2 that have > treeish1 as one of their ancestors which is definitely a valid > case for analysis, anyway). In this case, blame --reverse stops being > helpful. > At the cost of being slower, I just pushed to master the best results yet. The workaround I developed for the case I described on the previous mail ended up providing much better results overall so I ended up replacing the whole merge-analysis logic with it. Thanks for your kind help and comments, Peff. Let me know how it goes.
Re: difflame improvements
On Thu, Feb 16, 2017 at 11:17 PM, Jeff King wrote: > This isn't difflame's fault; that's what "git blame" tells you about > that line. But since I already told difflame "v2.6.5..HEAD", it would > probably make sense to similarly limit the blame to that range. That > turns up a boundary commit for the line. Which is _also_ not helpful, > but at least the tool is telling me that the line came from before > v2.6.5, and I don't really need to care much about it. I'm running my own tests on difflame and I have a theory about "when" it breaks at least one of the cases when it breaks: Analysis for deleted lines is being driven by git blame --reverse. What I have noticed is that it "breaks" when blame --reverse drives the analysis into revisions where "treeish1" is not part of their history (like, bringing analysis "to the sides" of treeish1 instead of keeping analysis in revisions in the history of treeish2 that have treeish1 as one of their ancestors which is definitely a valid case for analysis, anyway). In this case, blame --reverse stops being helpful. Take this example (I just pushed a debug-deletion branch into gh... probably more debugging messages will be needed): $ difflame.py HEAD~100 HEAD -- Documentation/git-commit.txt diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index f2ab0ee2e..4f8f20a36 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -265,7 +265,8 @@ FROM UPSTREAM REBASE" section in linkgit:git-rebase[1].) bcf9626a71 (Matthieu Moy 2016-06-28 13:40:11 +0200 265) If this option is specified together with `--amend`, then 04c8ce9c1c (Markus Heidelberg 2008-12-19 13:14:18 +0100 266) no paths need to be specified, which can be used to amend d4ba07cac5 (Johannes Sixt 2008-04-10 13:33:09 +0200 267) the last commit without committing changes that have Range of revisions: 02db2d..066fb04 Treeish1 02db2d04: 02db2d042 Merge branch 'ah/grammos' Treeish2 066fb0494: 066fb0494 blame: draft of line format Blamed Revision afe0e2a39: afe0e2a39 Merge branch 'da/difftool-dir-diff-fix' Original Filename a/Documentation/git-commit.txt Deleted Line 268 Children revisions: 3aead1cad7a: 3aead1cad Merge branch 'ak/commit-only-allow-empty' There's only one child revision on that revision the line we are tracking is gone Parents of this child revision: afe0e2a39166: afe0e2a39 Merge branch 'da/difftool-dir-diff-fix' beb635ca9ce: beb635ca9 commit: remove 'Clever' message for --only --amend Finding parent where the line has been deleted: beb635ca9: beb635ca9 commit: remove 'Clever' message for --only --amend Range of revisions: 02db2d042..beb635c Treeish1 02db2d0: 02db2d042 Merge branch 'ah/grammos' Treeish2 beb635c: beb635ca9 commit: remove 'Clever' message for --only --amend Blamed Revision 02db2d0: 02db2d042 Merge branch 'ah/grammos' Original Filename a/Documentation/git-commit.txt Deleted Line 268 Children revisions: Found no children... will return the original blamed revision (02db2d0) saying that the deleting revision could not be found beb635ca9 commit: remove 'Clever' message for --only --amend -beb635ca9 (Andreas Krey 2016-12-09 05:10:21 +0100 268) already been staged. 319d83524 commit: make --only --allow-empty work without paths +319d835240 (Andreas Krey 2016-12-02 23:15:13 +0100 268) already been staged. ... +319d835240 (Andreas Krey 2016-12-02 23:15:13 +0100 269) paths are also not requi... d4ba07cac5 (Johannes Sixt 2008-04-10 13:33:09 +0200 270) 1947bdbc31 (Junio C Hamano2008-06-22 14:32:27 -0700 271) -u[]:: 1947bdbc31 (Junio C Hamano2008-06-22 14:32:27 -0700 272) --untracked-files[=]:: I know that line 268 was deleted on 319d835240. So on the first round of merge analysis it says "let's go into beb635ca9". That's fine. That's exactly the path that is required to reach 319d835240. But then when using this new "range of revisions" for git blame --reverse, we get that line 268 is not telling us anything useful: $ git blame --reverse -L268,268 02db2d042..beb635c -- Documentation/git-commit.txt ^02db2d042 (Junio C Hamano 2016-12-19 14:45:30 -0800 268) already been staged. So, instead of pointing to 319d835240 (the parent of beb635c), it's basically saying something like "I give up". My hunch (haven't sat down to digest all the details about the output of git blame --reverse... YET) is that, given that 02db2d042 is _not_ part of the history of beb635c, git blame reverse is trying to tell me just that... and that means I'll have to "script around this scenario". $ git merge-base 02db2d042 beb635c 0202c411edc25940cc381bf317badcdf67670be4 Thanks in advance.
difflame improvements
Hi!
I've been working on detecting revisions where a "real" deletion was
made and I think I advanced a lot in that front. I still have to work
on many scenarios (renamed files, for example... also performance) but
at least I'm using a few runs against git-scm history and the results
are "promising":
23:05 $ git blame -s --reverse -L 25,40 HEAD~20..HEAD -- versioncmp.c
066fb0494e 25) static int initialized;
066fb0494e 26)
066fb0494e 27) /*
8ec68d1ae2 28) * p1 and p2 point to the first different character in
two strings. If
8ec68d1ae2 29) * either p1 or p2 starts with a prerelease suffix, it
will be forced
8ec68d1ae2 30) * to be on top.
8ec68d1ae2 31) *
8ec68d1ae2 32) * If both p1 and p2 start with (different) suffix, the order is
8ec68d1ae2 33) * determined by config file.
066fb0494e 34) *
8ec68d1ae2 35) * Note that we don't have to deal with the situation
when both p1 and
8ec68d1ae2 36) * p2 start with the same suffix because the common
part is already
8ec68d1ae2 37) * consumed by the caller.
066fb0494e 38) *
066fb0494e 39) * Return non-zero if *diff contains the return value
for versioncmp()
066fb0494e 40) */
Lines 28-33:
23:05 $ git show --summary 8ec68d1ae2
commit 8ec68d1ae2863823b74d67c5e92297e38bbf97bc
Merge: e801be066 c48886779
Author: Junio C Hamano <>
Date: Mon Jan 23 15:59:21 2017 -0800
Merge branch 'vn/diff-ihc-config'
"git diff" learned diff.interHunkContext configuration variable
that gives the default value for its --inter-hunk-context option.
* vn/diff-ihc-config:
diff: add interhunk context config option
And this is not telling me the _real_ revision where the lines were
_deleted_ so it's not very helpful, as Peff has already mentioned.
Running difflame:
23:06 $ time ~/proyectos/git/difflame/difflame.py -bp=-s -w HEAD~20
HEAD -- versioncmp.c
diff --git a/versioncmp.c b/versioncmp.c
index 80bfd109f..9f81dc106 100644
--- a/versioncmp.c
+++ b/versioncmp.c
@@ -24,42 +24,83 @@
.
.
.
+b17846432d 33) static void find_better_matching_suffix(const char
*tagname, const char *suffix,
+b17846432d 34)int
suffix_len, int start, int conf_pos,
+b17846432d 35)struct
suffix_match *match)
+b17846432d 36) {
b17846432d 37)/*
c026557a3 versioncmp: generalize version sort suffix reordering
-c026557a3 (SZEDER 28) * p1 and p2 point to the first different
character in two strings. If
-c026557a3 (SZEDER 29) * either p1 or p2 starts with a prerelease
suffix, it will be forced
-c026557a3 (SZEDER 30) * to be on top.
-c026557a3 (SZEDER 31) *
-c026557a3 (SZEDER 32) * If both p1 and p2 start with (different)
suffix, the order is
-c026557a3 (SZEDER 33) * determined by config file.
b17846432 versioncmp: factor out helper for suffix matching
+b17846432d 38) * A better match either starts earlier or
starts at the same offset
+b17846432d 39) * but is longer.
+b17846432d 40) */
+b17846432d 41)int end = match->len < suffix_len ?
match->start : match->start-1;
.
.
.
Same range of (deleted) lines:
23:10 $ git --show --name-status c026557a3
commit c026557a37361b7019acca28f240a19f546739e9
Author: SZEDER Gábor <>
Date: Thu Dec 8 15:24:01 2016 +0100
versioncmp: generalize version sort suffix reordering
The 'versionsort.prereleaseSuffix' configuration variable, as its name
suggests, is supposed to only deal with tagnames with prerelease
.
.
.
Signed-off-by: SZEDER Gábor <>
Signed-off-by: Junio C Hamano <>
M Documentation/config.txt
M Documentation/git-tag.txt
M t/t7004-tag.sh
M versioncmp.c
This is the revision where the deletion happened.
That's it for the time being.
Re: A little help understanding output from git blame --reverse
On Mon, Feb 6, 2017 at 6:38 AM, Edmundo Carmona Antoranz wrote: > I'm "difflaming" HEAD~100 (02db2d0421b97fcb6211) and HEAD > (066fb0494e6398eb). Specifically file abspath.c. I just noticed that I'm standing on a private branch. Replace HEAD for "4e59582ff" when doing your analysis. You should get the same results.
A little help understanding output from git blame --reverse
Hi!
While doing some research developing difflame I found some output from
git blame --reverse that I can't quite understand. Perhaps another set
of eyeballs could help me.
I'm "difflaming" HEAD~100 (02db2d0421b97fcb6211) and HEAD
(066fb0494e6398eb). Specifically file abspath.c.
If we diff (as in plain old git diff) HEAD~100..HEAD we can see that
line 63 (from HEAD~100 revision) was deleted between HEAD~100 and
HEAD:
@@ -58,86 +95,136 @@ blah blah
goto error_out;
}
- strbuf_reset(&sb);
- strbuf_addstr(&sb, path);
-
- while (depth--) {
- if (!is_directory(sb.buf)) {
So, if I do a "reverse" blame operation on the file, I would expect to
see the last revision where that line was _present_ on the file:
c5f3cba126 61) strbuf_reset(&sb);
c5f3cba126 62) strbuf_addstr(&sb, path);
066fb0494e 63)
c5f3cba126 64) while (depth--) {
c5f3cba126 65) if (!is_directory(sb.buf)) {
line 63 shows up as if it had been last present on the file on
revision 066fb0494e, which is HEAD, which kind of doesn't make a lot
of sense to me because given that the line is not present on the file
on HEAD (as we can guess from diff output) it means it was "forcefully
present" on some previous revision (and that's what I would expect to
see reported on blame --reverse output).
What am I missing? Thanks in advance.
Re: difflame
On Thu, Feb 2, 2017 at 10:46 PM, Edmundo Carmona Antoranz wrote: > I have been "scripting around git blame --reverse" for some days now. > Mind taking a look? I've been working on blame-deletions for this. blame-deletions branch, that is Sorry for the previous top-posting.
Re: difflame
I have been "scripting around git blame --reverse" for some days now. Mind taking a look? I've been working on blame-deletions for this. 22:41 $ ../difflame/difflame.py HEAD~5 HEAD diff --git a/file b/file index b414108..051c298 100644 --- a/file +++ b/file @@ -1,6 +1,9 @@ ^1513353 (Edmundo 2017-02-02 22:41:45 -0600 1) 1 f20952eb (Edmundo 2017-02-02 22:41:45 -0600 2) 2 bb04dc7c (Edmundo 2017-02-02 22:41:45 -0600 3) 3 -de3c07b (Edmundo 2017-02-02 22:41:47 -060 4) 4 058ea125 (Edmundo 2017-02-02 22:41:45 -0600 4) 5 85fc6b81 (Edmundo 2017-02-02 22:41:45 -0600 5) 6 +2cd990a6 (Edmundo 2017-02-02 22:41:45 -0600 6) 7 +ab0be970 (Edmundo 2017-02-02 22:41:45 -0600 7) 8 +944452c0 (Edmundo 2017-02-02 22:41:45 -0600 8) 9 +6641edb0 (Edmundo 2017-02-02 22:41:45 -0600 9) 10 $ git show de3c07b commit de3c07bc21a83472d5c5ddf172dcb742665924dd (HEAD -> master) Author: Edmundo Date: Thu Feb 2 22:41:47 2017 -0600 drop 4 diff --git a/file b/file index f00c965..051c298 100644 --- a/file +++ b/file @@ -1,7 +1,6 @@ 1 2 3 -4 5 6 7 Next step: solve the find-real-deletion-revision-when-there-are-multiple-child-nodes problem and let me read the discussion around git blame --reverse. Thanks in advance. On Mon, Jan 30, 2017 at 8:37 PM, Edmundo Carmona Antoranz wrote: > Maybe a little work on blame to get the actual revision where some > lines were "deleted"? > > Something like git blame --blame-deletion that could be based on --reverse? > > On Mon, Jan 30, 2017 at 7:35 PM, Edmundo Carmona Antoranz > wrote: >> I'm thinking of something like this: >> >> Say I just discovered a problem in a file I want to see who worked >> on it since some revision that I know is working fine (or even >> something as generic as HEAD~100..). It could be a number of people >> with different revisions. I would diff to see what changed and >> blame the added lines (blame reverse to try to get as close as >> possible with a single command in case I want to see what happened >> with something that was deleted). If I could get blame information of >> added/deleted lines in a single run, that would help a lot. >> >> Lo and behold: difflame. >> >> >> >> On Mon, Jan 30, 2017 at 5:26 PM, Jeff King wrote: >>> On Mon, Jan 30, 2017 at 01:08:41PM -0800, Junio C Hamano wrote: >>> >>>> Jeff King writes: >>>> >>>> > On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote: >>>> > >>>> >> For a very long time I had wanted to get the output of diff to include >>>> >> blame information as well (to see when something was added/removed). >>>> > >>>> > This is something I've wanted, too. The trickiest part, though, is >>>> > blaming deletions, because git-blame only tracks the origin of content, >>>> > not the origin of a change. >>>> >>>> Hmph, this is a comment without looking at what difflame does >>>> internally, so you can ignore me if I am misunderstood what problem >>>> you are pointing out, but I am not sure how "tracks the origin of >>>> content" could be a problem. >>>> >>>> If output from "git show" says this: >>>> >>>> --- a/file >>>> +++ b/file >>>> @@ -1,5 +1,6 @@ >>>>a >>>>b >>>> -c >>>> +C >>>> +D >>>>d >>>>e >>>> >>>> in order to annotate lines 'a', 'b', 'd', and 'e' for their origin, >>>> you would run 'blame' on the commit the above output was taken from >>>> (or its parent---they are in the context so either would be OK). >>>> >>>> You know where 'C' and 'D' came from already. It's the commit you >>>> are feeding "git show". >>> >>> I think the point (or at least as I understand it) is that the diff is >>> not "git show" for a particular commit. It could be part of a much >>> larger diff that covers many commits. >>> >>> As a non-hypothetical instance, I have a fork of git.git that has >>> various enhancements. I want to feed those enhancements upstream. I need >>> to know which commits should be cherry-picked to get those various >>> enhancements. >>> >>> Looking at "log origin..fork" tells me too many commits, because it >>> includes ones which aren't useful anymore. Either because they already >
Re: difflame
Maybe a little work on blame to get the actual revision where some lines were "deleted"? Something like git blame --blame-deletion that could be based on --reverse? On Mon, Jan 30, 2017 at 7:35 PM, Edmundo Carmona Antoranz wrote: > I'm thinking of something like this: > > Say I just discovered a problem in a file I want to see who worked > on it since some revision that I know is working fine (or even > something as generic as HEAD~100..). It could be a number of people > with different revisions. I would diff to see what changed and > blame the added lines (blame reverse to try to get as close as > possible with a single command in case I want to see what happened > with something that was deleted). If I could get blame information of > added/deleted lines in a single run, that would help a lot. > > Lo and behold: difflame. > > > > On Mon, Jan 30, 2017 at 5:26 PM, Jeff King wrote: >> On Mon, Jan 30, 2017 at 01:08:41PM -0800, Junio C Hamano wrote: >> >>> Jeff King writes: >>> >>> > On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote: >>> > >>> >> For a very long time I had wanted to get the output of diff to include >>> >> blame information as well (to see when something was added/removed). >>> > >>> > This is something I've wanted, too. The trickiest part, though, is >>> > blaming deletions, because git-blame only tracks the origin of content, >>> > not the origin of a change. >>> >>> Hmph, this is a comment without looking at what difflame does >>> internally, so you can ignore me if I am misunderstood what problem >>> you are pointing out, but I am not sure how "tracks the origin of >>> content" could be a problem. >>> >>> If output from "git show" says this: >>> >>> --- a/file >>> +++ b/file >>> @@ -1,5 +1,6 @@ >>>a >>>b >>> -c >>> +C >>> +D >>>d >>>e >>> >>> in order to annotate lines 'a', 'b', 'd', and 'e' for their origin, >>> you would run 'blame' on the commit the above output was taken from >>> (or its parent---they are in the context so either would be OK). >>> >>> You know where 'C' and 'D' came from already. It's the commit you >>> are feeding "git show". >> >> I think the point (or at least as I understand it) is that the diff is >> not "git show" for a particular commit. It could be part of a much >> larger diff that covers many commits. >> >> As a non-hypothetical instance, I have a fork of git.git that has >> various enhancements. I want to feed those enhancements upstream. I need >> to know which commits should be cherry-picked to get those various >> enhancements. >> >> Looking at "log origin..fork" tells me too many commits, because it >> includes ones which aren't useful anymore. Either because they already >> went upstream, or because they were cherry-picked to the fork and their >> upstream counterparts merged (or even equivalent commits made upstream >> that obsoleted the features). >> >> Looking at "git diff origin fork" tells me what the actual differences >> are, but it doesn't show me which commits are responsible for them. >> >> I can "git blame" each individual line of the diff (starting with "fork" >> as the tip), but that doesn't work for lines that no longer exist (i.e., >> when the interesting change is a deletion). >> >>> In order to run blame to find where 'c' came from, you need to start >>> at the _parent_ of the commit the above output came from, and the >>> hunk header shows which line range to find the final 'c'. >> >> So perhaps that explains my comment more. "blame" is not good for >> finding which commit took away a line. I've tried using "blame >> --reverse", but it shows you the parent of the commit you are looking >> for, which is slightly annoying. :) >> >> "git log -S" is probably a better tool for finding that. >> >> -Peff
Re: [PATCH] blame: draft of line format
I'm basing in on the "pretty API" so that we don't have to reinvent the wheel. I have already noticed that many of the formatting options available for pretty are not working... I'm sure it would require some work setting up the call to pretty api but the basic is laid out there. Let me know of your thoughts.
[PATCH] blame: draft of line format
---
builtin/blame.c | 24
1 file changed, 24 insertions(+)
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..89c1a862d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -52,6 +52,7 @@ static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
static int show_progress;
+static char *format_line;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -1931,6 +1932,19 @@ static void emit_porcelain(struct scoreboard *sb, struct
blame_entry *ent,
putchar('\n');
}
+static void pretty_info(char* revid, struct blame_entry *ent, struct strbuf
*rev_buffer)
+{
+ struct pretty_print_context ctx = {0};
+ struct rev_info rev;
+
+ struct strbuf format = STRBUF_INIT;
+ strbuf_addstr(&format, format_line);
+ ctx.fmt = CMIT_FMT_USERFORMAT;
+ get_commit_format(format.buf, &rev);
+ pretty_print_commit(&ctx, ent->suspect->commit, rev_buffer);
+ strbuf_release(&format);
+}
+
static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
{
int cnt;
@@ -1939,11 +1953,15 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
struct commit_info ci;
char hex[GIT_SHA1_HEXSZ + 1];
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+ struct strbuf line_revision_buf = STRBUF_INIT;
get_commit_info(suspect->commit, &ci, 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
cp = nth_line(sb, ent->lno);
+
+ if (format_line)
+ pretty_info(hex, ent, &line_revision_buf);
for (cnt = 0; cnt < ent->num_lines; cnt++) {
char ch;
int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ :
abbrev;
@@ -1968,6 +1986,10 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
format_time(ci.author_time, ci.author_tz.buf,
show_raw_time),
ent->lno + 1 + cnt);
+ } else if (format_line) {
+ printf("%s", line_revision_buf.buf);
+ printf(" %*d) ",
+ max_digits, ent->lno + 1 + cnt);
} else {
if (opt & OUTPUT_SHOW_SCORE)
printf(" %*d %02d",
@@ -2007,6 +2029,7 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
if (sb->final_buf_size && cp[-1] != '\n')
putchar('\n');
+ strbuf_release(&line_revision_buf);
commit_info_destroy(&ci);
}
@@ -2605,6 +2628,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BIT(0, "minimal", &xdl_opts, N_("Spend extra cycles to find
better match"), XDF_NEED_MINIMAL),
OPT_STRING('S', NULL, &revs_file, N_("file"), N_("Use revisions
from instead of calling git-rev-list")),
+ OPT_STRING(0, "format-line", &format_line, N_("format-line"),
N_("Use pretty format for revisions")),
OPT_STRING(0, "contents", &contents_from, N_("file"), N_("Use
's contents as the final image")),
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line
copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line
movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
--
2.11.0.rc1
Re: difflame
I'm thinking of something like this: Say I just discovered a problem in a file I want to see who worked on it since some revision that I know is working fine (or even something as generic as HEAD~100..). It could be a number of people with different revisions. I would diff to see what changed and blame the added lines (blame reverse to try to get as close as possible with a single command in case I want to see what happened with something that was deleted). If I could get blame information of added/deleted lines in a single run, that would help a lot. Lo and behold: difflame. On Mon, Jan 30, 2017 at 5:26 PM, Jeff King wrote: > On Mon, Jan 30, 2017 at 01:08:41PM -0800, Junio C Hamano wrote: > >> Jeff King writes: >> >> > On Tue, Jan 17, 2017 at 11:24:02PM -0600, Edmundo Carmona Antoranz wrote: >> > >> >> For a very long time I had wanted to get the output of diff to include >> >> blame information as well (to see when something was added/removed). >> > >> > This is something I've wanted, too. The trickiest part, though, is >> > blaming deletions, because git-blame only tracks the origin of content, >> > not the origin of a change. >> >> Hmph, this is a comment without looking at what difflame does >> internally, so you can ignore me if I am misunderstood what problem >> you are pointing out, but I am not sure how "tracks the origin of >> content" could be a problem. >> >> If output from "git show" says this: >> >> --- a/file >> +++ b/file >> @@ -1,5 +1,6 @@ >>a >>b >> -c >> +C >> +D >>d >>e >> >> in order to annotate lines 'a', 'b', 'd', and 'e' for their origin, >> you would run 'blame' on the commit the above output was taken from >> (or its parent---they are in the context so either would be OK). >> >> You know where 'C' and 'D' came from already. It's the commit you >> are feeding "git show". > > I think the point (or at least as I understand it) is that the diff is > not "git show" for a particular commit. It could be part of a much > larger diff that covers many commits. > > As a non-hypothetical instance, I have a fork of git.git that has > various enhancements. I want to feed those enhancements upstream. I need > to know which commits should be cherry-picked to get those various > enhancements. > > Looking at "log origin..fork" tells me too many commits, because it > includes ones which aren't useful anymore. Either because they already > went upstream, or because they were cherry-picked to the fork and their > upstream counterparts merged (or even equivalent commits made upstream > that obsoleted the features). > > Looking at "git diff origin fork" tells me what the actual differences > are, but it doesn't show me which commits are responsible for them. > > I can "git blame" each individual line of the diff (starting with "fork" > as the tip), but that doesn't work for lines that no longer exist (i.e., > when the interesting change is a deletion). > >> In order to run blame to find where 'c' came from, you need to start >> at the _parent_ of the commit the above output came from, and the >> hunk header shows which line range to find the final 'c'. > > So perhaps that explains my comment more. "blame" is not good for > finding which commit took away a line. I've tried using "blame > --reverse", but it shows you the parent of the commit you are looking > for, which is slightly annoying. :) > > "git log -S" is probably a better tool for finding that. > > -Peff
[PATCH v1 2/3] blame: add --hint option
When printing aggregate information about revisions, this option
allows to add the 1-line summary of the revision to provide the
reader with some additional context about the revision itself.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 4
Documentation/git-blame.txt | 2 +-
builtin/blame.c | 13 +++--
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index be2a327ff..64858a631 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -53,6 +53,10 @@ include::line-range-format.txt[]
Aggregate information about revisions. This option makes no
difference on incremental or porcelain format.
+--hint::
+ Show revision hints on aggregated information about the revision.
+ Implies --aggregate.
+
--encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 385eaf7be..ed8b119fa 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--progress] [--abbrev=] [--aggregate]
+ [--progress] [--abbrev=] [--aggregate] [--hint]
[ | --contents | --reverse ..] [--]
DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 09b3b0c8a..7473b50e9 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1885,6 +1885,7 @@ static const char *format_time(unsigned long time, const
char *tz_str,
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
#define OUTPUT_AGGREGATE 02000
+#define OUTPUT_HINT 04000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -2001,8 +2002,12 @@ static void print_revision_info(char* revision_hex, int
revision_length, struct
if (line_number && (opt & OUTPUT_AGGREGATE))
printf(opt & OUTPUT_ANNOTATE_COMPAT ? "%*d)" : " %*d) ",
max_digits, line_number);
- if (!line_number)
- printf(")\n");
+ if (!line_number) {
+ printf(")");
+ if (opt & OUTPUT_HINT)
+ printf(" %s", ci.summary.buf);
+ printf("\n");
+ }
}
static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2018,6 +2023,9 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
get_commit_info(suspect->commit, &ci, 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ if (~(opt & OUTPUT_AGGREGATE) & (opt & OUTPUT_HINT))
+ opt=opt|OUTPUT_AGGREGATE;
+
if (opt & OUTPUT_AGGREGATE)
print_revision_info(hex, revision_length, ent, ci, opt,
show_raw_time, 0);
@@ -2638,6 +2646,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line
movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process
only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"),
OUTPUT_AGGREGATE),
+ OPT_BIT(0, "hint", &output_option, N_("Show revision hints"),
OUTPUT_HINT),
OPT__ABBREV(&abbrev),
OPT_END()
};
--
2.11.0.rc1
[PATCH v1 3/3] blame: add --color option
Revision information will be highlighted so that it's easier
to tell from content of the file being "blamed".
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 5 +
Documentation/git-blame.txt | 2 +-
builtin/blame.c | 13 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 64858a631..4abb4eb7e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -57,6 +57,11 @@ include::line-range-format.txt[]
Show revision hints on aggregated information about the revision.
Implies --aggregate.
+--[no-]color::
+ Revision information will be highlighted on normal output
+ when running git from a terminal. This option allows
+ for color to be forcibly enabled or disabled.
+
--encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index ed8b119fa..d25cbc5ef 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,7 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--progress] [--abbrev=] [--aggregate] [--hint]
+ [--progress] [--abbrev=] [--aggregate] [--hint] [--color]
[ | --contents | --reverse ..] [--]
DESCRIPTION
diff --git a/builtin/blame.c b/builtin/blame.c
index 7473b50e9..c661dd538 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1886,6 +1886,7 @@ static const char *format_time(unsigned long time, const
char *tz_str,
#define OUTPUT_LINE_PORCELAIN 01000
#define OUTPUT_AGGREGATE 02000
#define OUTPUT_HINT 04000
+#define OUTPUT_COLOR 01
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1943,6 +1944,8 @@ static void emit_porcelain(struct scoreboard *sb, struct
blame_entry *ent,
static void print_revision_info(char* revision_hex, int revision_length,
struct blame_entry* ent,
struct commit_info ci, int opt, int show_raw_time, int
line_number)
{
+ if (opt & OUTPUT_COLOR)
+ printf("%s", GIT_COLOR_BOLD);
if (!line_number)
printf("\t");
int length = revision_length;
@@ -2008,6 +2011,8 @@ static void print_revision_info(char* revision_hex, int
revision_length, struct
printf(" %s", ci.summary.buf);
printf("\n");
}
+ if (opt & OUTPUT_COLOR)
+ printf("%s", GIT_COLOR_RESET);
}
static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
@@ -2608,6 +2613,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
char *final_commit_name = NULL;
enum object_type type;
struct commit *final_commit = NULL;
+ int show_color;
struct string_list range_list = STRING_LIST_INIT_NODUP;
int output_option = 0, opt = 0;
@@ -2647,6 +2653,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process
only line range n,m, counting from 1")),
OPT_BIT(0, "aggregate", &output_option, N_("Aggregate output"),
OUTPUT_AGGREGATE),
OPT_BIT(0, "hint", &output_option, N_("Show revision hints"),
OUTPUT_HINT),
+ OPT_BOOL(0, "color", &show_color, N_("Show colors on revision
information")),
OPT__ABBREV(&abbrev),
OPT_END()
};
@@ -2666,6 +2673,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
save_commit_buffer = 0;
dashdash_pos = 0;
show_progress = -1;
+ show_color = -1;
parse_options_start(&ctx, argc, argv, prefix, options,
PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
@@ -2698,6 +2706,11 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
} else if (show_progress < 0)
show_progress = isatty(2);
+ if (show_color < 0)
+ show_color = isatty(1);
+ if (show_color)
+ output_option = output_option | OUTPUT_COLOR;
+
if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
/* one more abbrev length is needed for the boundary commit */
abbrev++;
--
2.11.0.rc1
[PATCH v1 1/3] blame: add --aggregate option
To avoid taking up so much space on normal output printing duplicate
information on consecutive lines that "belong" to the same revision,
this option allows to print a single line with the information about
the revision before the lines of content themselves.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 4 ++
Documentation/git-blame.txt | 4 +-
builtin/blame.c | 85 +++--
3 files changed, 63 insertions(+), 30 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 2669b87c9..be2a327ff 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -49,6 +49,10 @@ include::line-range-format.txt[]
Show the result incrementally in a format designed for
machine consumption.
+--aggregate::
+ Aggregate information about revisions. This option makes no
+ difference on incremental or porcelain format.
+
--encoding=::
Specifies the encoding used to output author names
and commit summaries. Setting it to `none` makes blame
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index fdc3aea30..385eaf7be 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,8 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--progress] [--abbrev=] [ | --contents | --reverse
..]
- [--]
+ [--progress] [--abbrev=] [--aggregate]
+ [ | --contents | --reverse ..] [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..09b3b0c8a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const
char *tz_str,
#define OUTPUT_NO_AUTHOR 0200
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_AGGREGATE 02000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1931,43 +1932,41 @@ static void emit_porcelain(struct scoreboard *sb,
struct blame_entry *ent,
putchar('\n');
}
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
-{
- int cnt;
- const char *cp;
- struct origin *suspect = ent->suspect;
- struct commit_info ci;
- char hex[GIT_SHA1_HEXSZ + 1];
- int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
-
- get_commit_info(suspect->commit, &ci, 1);
- sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
-
- cp = nth_line(sb, ent->lno);
- for (cnt = 0; cnt < ent->num_lines; cnt++) {
- char ch;
- int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ :
abbrev;
-
- if (suspect->commit->object.flags & UNINTERESTING) {
+/**
+ * Print information about the revision.
+ * This information can be used in either aggregated output
+ * or prepending each line of the content of the file being blamed
+ *
+ * if line_number == 0, it's an aggregate line
+ */
+static void print_revision_info(char* revision_hex, int revision_length,
struct blame_entry* ent,
+ struct commit_info ci, int opt, int show_raw_time, int
line_number)
+{
+ if (!line_number)
+ printf("\t");
+ int length = revision_length;
+ if (!line_number || !(opt & OUTPUT_AGGREGATE)) {
+ if (ent->suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
- memset(hex, ' ', length);
+ memset(revision_hex, ' ', length);
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--;
putchar('^');
}
}
- printf("%.*s", length, hex);
+ printf("%.*s", length, revision_hex);
if (opt & OUTPUT_ANNOTATE_COMPAT) {
const char *name;
if (opt & OUTPUT_SHOW_EMAIL)
name = ci.author_mail.buf;
else
name = ci.author.buf;
- printf("\t(%10s\t%10s\t%d)", name,
+ printf("\t(%10s\t%10s", name,
format_time(ci.author_time, ci.author_tz.buf,
- show_raw_time),
- ent->lno + 1 + cnt);
+ show_raw_time));
+ if (line_number)
+
Re: [PATCH] [draft]blame: add --aggregate option
Developers of the world, rejoice! :-)
Junio, Pranit (and whoever is paying attention to the conversation
that was being held about --tips), here's a draft of what I meant when
I was talking about the option of "aggregating" blame output. I'm not
considering _all_ cases yet, just would like for people to give it a
quick test and tell me if they think it's worth "polishing" it for
inclusion into mainline git.
The output would look like this:
$ ./git blame -L 1,19 -t --aggregate builtin/blame.c
Blaming lines: 0% (19/2974), done.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 1161298804 -0700)
1) /*
31653c1abc builtin-blame.c (Eugene Letuchy 1235170271 -0800)
2) * Blame
cee7f245dc builtin-pickaxe.c (Junio C Hamano 1161298804 -0700)
3) *
7e6ac6e439 builtin/blame.c (David Kastrup1398470209 +0200)
4) * Copyright (c) 2006, 2014 by its authors
5) * See COPYING for licensing conditions
cee7f245dc builtin-pickaxe.c (Junio C Hamano 1161298804 -0700)
6) */
7)
8) #include "cache.h"
fb58c8d507 builtin/blame.c (Michael Haggerty 1434981785 +0200)
9) #include "refs.h"
cee7f245dc builtin-pickaxe.c (Junio C Hamano 1161298804 -0700)
10) #include "builtin.h"
11) #include "blob.h"
12) #include "commit.h"
13) #include "tag.h"
14) #include "tree-walk.h"
15) #include "diff.h"
16) #include "diffcore.h"
17) #include "revision.h"
717d1462ba builtin-blame.c (Linus Torvalds 1169976846 -0800)
18) #include "quote.h"
cee7f245dc builtin-pickaxe.c (Junio C Hamano 1161298804 -0700)
19) #include "xdiff-interface.h"
It can be seen that options like -t still work in aggregation.
In relation to the previous conversation about "tips", I think a
better name could be "hints" and it could be added on top of the
aggregation.
On Mon, Jan 23, 2017 at 8:10 PM, Edmundo Carmona Antoranz
wrote:
> ---
> builtin/blame.c | 78
> +
> 1 file changed, 51 insertions(+), 27 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 126b8c9e5..9e8403303 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time,
> const char *tz_str,
> #define OUTPUT_NO_AUTHOR 0200
> #define OUTPUT_SHOW_EMAIL 0400
> #define OUTPUT_LINE_PORCELAIN 01000
> +#define OUTPUT_AGGREGATE 02000
>
> static void emit_porcelain_details(struct origin *suspect, int repeat)
> {
> @@ -1931,43 +1932,36 @@ static void emit_porcelain(struct scoreboard *sb,
> struct blame_entry *ent,
> putchar('\n');
> }
>
> -static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int
> opt)
> +/**
> + * Print information about the revision.
> + * This information can be used in either aggregated output
> + * or prepending each line of the content of the file being blamed
> + */
> +static void print_revision_info(char* revision_hex, int revision_length,
> struct blame_entry* ent,
> + struct commit* commit, struct commit_info ci, int opt, int
> show_raw_time)
> {
> - int cnt;
> - const char *cp;
> - struct origin *suspect = ent->suspect;
> - struct commit_info ci;
> - char hex[GIT_SHA1_HEXSZ + 1];
> - int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
> -
> - get_commit_info(suspect->commit, &ci, 1);
> - sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
> -
> - cp = nth_line(sb, ent->lno);
> - for (cnt = 0; cnt < ent->num_lines; cnt++) {
> - char ch;
> - int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ
> : abbrev;
> -
> - if (suspect->commit->object.flags & UNINTERESTING) {
> + if (opt & OUTPUT_AGGREGATE)
> + printf("\t");
> + int length = revision_length;
> + if (commit->object.flags & UNINTERESTING) {
> if (blank_boundary)
> - memset(hex, ' ', length);
> + memset(revision_hex, ' ', length);
> else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
> length--;
> putchar('^');
> }
> }
>
> - printf("%.*s", length, hex);
> + printf("%.*s", length, revision_hex);
> if (opt & OUTPUT_ANNOTATE_CO
[PATCH] [draft]blame: add --aggregate option
---
builtin/blame.c | 78 +
1 file changed, 51 insertions(+), 27 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..9e8403303 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const
char *tz_str,
#define OUTPUT_NO_AUTHOR 0200
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_AGGREGATE 02000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1931,43 +1932,36 @@ static void emit_porcelain(struct scoreboard *sb,
struct blame_entry *ent,
putchar('\n');
}
-static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+/**
+ * Print information about the revision.
+ * This information can be used in either aggregated output
+ * or prepending each line of the content of the file being blamed
+ */
+static void print_revision_info(char* revision_hex, int revision_length,
struct blame_entry* ent,
+ struct commit* commit, struct commit_info ci, int opt, int
show_raw_time)
{
- int cnt;
- const char *cp;
- struct origin *suspect = ent->suspect;
- struct commit_info ci;
- char hex[GIT_SHA1_HEXSZ + 1];
- int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
-
- get_commit_info(suspect->commit, &ci, 1);
- sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
-
- cp = nth_line(sb, ent->lno);
- for (cnt = 0; cnt < ent->num_lines; cnt++) {
- char ch;
- int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ :
abbrev;
-
- if (suspect->commit->object.flags & UNINTERESTING) {
+ if (opt & OUTPUT_AGGREGATE)
+ printf("\t");
+ int length = revision_length;
+ if (commit->object.flags & UNINTERESTING) {
if (blank_boundary)
- memset(hex, ' ', length);
+ memset(revision_hex, ' ', length);
else if (!(opt & OUTPUT_ANNOTATE_COMPAT)) {
length--;
putchar('^');
}
}
- printf("%.*s", length, hex);
+ printf("%.*s", length, revision_hex);
if (opt & OUTPUT_ANNOTATE_COMPAT) {
const char *name;
if (opt & OUTPUT_SHOW_EMAIL)
name = ci.author_mail.buf;
else
name = ci.author.buf;
- printf("\t(%10s\t%10s\t%d)", name,
+ printf("\t(%10s\t%10s\t", name,
format_time(ci.author_time, ci.author_tz.buf,
- show_raw_time),
- ent->lno + 1 + cnt);
+ show_raw_time));
} else {
if (opt & OUTPUT_SHOW_SCORE)
printf(" %*d %02d",
@@ -1975,11 +1969,7 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
ent->suspect->refcnt);
if (opt & OUTPUT_SHOW_NAME)
printf(" %-*.*s", longest_file, longest_file,
- suspect->path);
- if (opt & OUTPUT_SHOW_NUMBER)
- printf(" %*d", max_orig_digits,
- ent->s_lno + 1 + cnt);
-
+ ent->suspect->path);
if (!(opt & OUTPUT_NO_AUTHOR)) {
const char *name;
int pad;
@@ -1994,9 +1984,42 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
ci.author_tz.buf,
show_raw_time));
}
+ }
+ if (opt & OUTPUT_AGGREGATE)
+ printf(")\n");
+}
+
+static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
+{
+ int cnt;
+ const char *cp;
+ struct origin *suspect = ent->suspect;
+ struct commit_info ci;
+ char hex[GIT_SHA1_HEXSZ + 1];
+ int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+ int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ
: abbrev;
+
+ get_commit_info(suspect->commit, &ci, 1);
+ sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+
+ if (opt & OUTPUT_AGGREGATE)
+ print_revision_info(hex, revision_length, ent, suspect->commit,
ci, opt, show_raw_time);
+
+ cp = nth_line(sb, ent-
Re: [PATCH] blame: add option to print tips (--tips)
On Sun, Jan 22, 2017 at 4:58 PM, Junio C Hamano wrote: > > What is the target audience? If you are trying to write a script > that reads output by "git blame", you are strongly discouraged > unless you are reading from "git blame --porcelain" which is more > compact and has this information already IIRC. I wrote this for human consumption, actually. Maybe I overestimated the need for this feature (I definitely find it handy, but it might be just me).
Re: Does it make sense to show tips on blame?
On Sun, Jan 22, 2017 at 4:25 PM, Junio C Hamano wrote:
> Edmundo Carmona Antoranz writes:
>
> What does the word "tip" mean in this context? The word is often
> used to mean the commits directly pointed at by branches (i.e. the
> tip of history), but I do not think that is what you are interested
> in showing in this butchered "diff" output.
>
> Do you mean the one-line summary for the commit? The commits that
> are shown will not be at the tip most of the time (the "+" ones may
> be if you happen to be running "git show" on the tip of a branch,
> but that is minority if you also want to do "git log -p"), so I am
> not sure it makes sense to call them "tips" in the above output.
>
Yeah, it would be the 1-line summary for the revision that is related
to the lines that
will be printed right after the summary, as a way to provide
additional context about
those lines without having to resort to an additional git show
--summary command.
Perhaps a name more appropriate name than "tips" in the context of git would be
better suited to avoid confusion.
> If I were doing the above, I would probably do them as footnotes to
> keep the body of the patch text (slightly) more readable. E.g.
>
> @@ -l,k +m,n @@
> 2a113aee9b
> +405d7f4af6 if (!root-tree)
> +405d7f4af6 load_tree(root)
> ...
> #2a113aee9b "fast-import: Proper notes tree manipulation", 2009-12-07
> #405d7f4af6 "fast-import: properly fanout notes when tree is
> imported", 2016-12-21
>
Now, this is a different topic altogether (difflame related) and I
think it was raised from the fact that I
included output from difflame (which starts from diff output) as my
example. When talking about
blame it would be no-patches, just plain blame output writing the
1-line summary for blocks of lines
related to the same revision. I already sent a patch today with the
changes needed to see what I mean in action.
Could you give it a test drive? Consider it's a rough draft (and I
also raise the question of "aggregating" output
on the mail thread)
Something like this, hope this doesn't get butchered (at least not
that hard, i'm truncating the lines):
3d426842: README.txt
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 1) difflame
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 2)
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 3) Copyright 2017
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 4) Released unde
3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 5)
0660083e: Add params to blame and diff, adjust README.txt
0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600 6) Show the outp
0660083e (Edmundo Carmona Antoranz 2017-01-18 20:51:12 -0600 7)
c869edc9: README.txt - stick (or at least, at least try) to 80 columns
c869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600 8) Example outp
c869edc9 (Edmundo Carmona Antoranz 2017-01-20 23:39:51 -0600 9) params to cha
>> The question is if you think it makes sense to add this option to
>> git-blame itself.
>>
>> Something like:
>> git blame --tips README.txt
>
> I do not think I would use it personally, as it would make it hard
> to pipe the output of "git blame" to less and then /search things,
> as extra cruft added by the option will get in the way. IOW, I do
> not think we want it for human-oriented output.
Doing a search through less, it would make sense to leave the 1-line summary out
but when looking at blame output through OSI layer 8, I'd definitely
like to get that 1-line summary "as an option" every so often (more
often than not
in my case, probably).
Re: [PATCH] blame: add option to print tips (--tips)
Hello, everybody!
So, this is a draft of what I mean by "adding tips to blame".
Example output (sample from builtin/blame.c):
15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1934)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1935) static void emit_other(struct scoreboard *sb,
struct blame_entry *ent, int opt)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1936) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1937) int cnt;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1938) const char *cp;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1939) struct origin *suspect = ent->suspect;
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1940) struct commit_info ci;
d59f765ac9: use sha1_to_hex_r() instead of strcpy
d59f765ac9 builtin/blame.c (Jeff King2015-09-24
17:08:03 -0400 1941) char hex[GIT_SHA1_HEXSZ + 1];
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1942) int show_raw_time = !!(opt &
OUTPUT_RAW_TIMESTAMP);
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1943) int revision_length = (opt &
OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ : abbrev;
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1944)
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1945) get_commit_info(suspect->commit, &ci, 1);
f2fd0760f6: Convert struct object to object_id
f2fd0760f6 builtin/blame.c (brian m. carlson 2015-11-10
02:22:28 + 1946) sha1_to_hex_r(hex,
suspect->commit->object.oid.hash);
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1947)
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1948) if (opt & OUTPUT_SHOW_TIPS)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1949) printf("\t%.*s: %s\n", revision_length,
hex, ci.summary.buf);
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1950)
cee7f245dc: git-pickaxe: blame rewritten.
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1951) cp = nth_line(sb, ent->lno);
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1952) for (cnt = 0; cnt < ent->num_lines; cnt++) {
cee7f245dc builtin-pickaxe.c (Junio C Hamano 2006-10-19
16:00:04 -0700 1953) char ch;
f2aea1696f: blame: add option to print tips (--tips)
f2aea1696f builtin/blame.c (Edmundo Carmona Antoranz 2017-01-22
15:23:31 -0600 1954) int length = revision_length;
b11121d9e3: git-blame: show lines attributed to boundary
commits differently.
b11121d9e3 builtin-blame.c (Junio C Hamano 2006-12-01
20:45:45 -0800 1955)
b11121d9e3 builtin-blame.c (Junio C Hamano 2006-12-01
20:45:45 -0800 1956) if (suspect->commit->object.flags &
UNINTERESTING) {
e68989a739: annotate: fix for cvsserver.
e68989a739 builtin-blame.c (Junio C Hamano 2007-02-06
01:52:04 -0800 1957) if (blank_boundary)
e68989a739 builtin-blame.c (Junio C Hamano 2007-02-06
01:52:04 -0800 1958) memset(hex, ' ',
length);
7ceacdffc5: "blame -c" should be compatible with "annotate"
7ceacdffc5 builtin-blame.c (Junio C Hamano 2008-09-05
00:57:35 -0700 1959) else if (!(opt &
OUTPUT_ANNOTATE_COMPAT)) {
4c10a5caa7: blame: -b (blame.blankboundary) and --root (blame.showroot)
4c10a5caa7 builtin-blame.c (Junio C Hamano 2006-12-18
14:04:38 -0800 1960) length--;
Does it look "worthy"? And if so, would it be better to think of
something like an "aggregate" option (or something like that) that
would include the common information as tips, something like:
15:32 $ ./git blame --tips -L 1934,1960 builtin/blame.c
cee7f245dc: builtin-pickaxe.c (Junio C Hamano
2006-10-19 16:00:04 -0700) git-pickaxe: blame rewritten.
1934
1935 static void emit_other(struct scoreboard *sb, struct blame_entry
*ent, int opt)
1936 {
1937 int cnt;
1938 const char *cp;
1939 struct origin *suspect = ent->suspect;
1940 struct commit_info ci;
d59f765ac9: bui
[PATCH] blame: add option to print tips (--tips)
---
builtin/blame.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 126b8c9e5..4bc449f40 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1884,6 +1884,7 @@ static const char *format_time(unsigned long time, const
char *tz_str,
#define OUTPUT_NO_AUTHOR 0200
#define OUTPUT_SHOW_EMAIL 0400
#define OUTPUT_LINE_PORCELAIN 01000
+#define OUTPUT_SHOW_TIPS 02000
static void emit_porcelain_details(struct origin *suspect, int repeat)
{
@@ -1939,14 +1940,18 @@ static void emit_other(struct scoreboard *sb, struct
blame_entry *ent, int opt)
struct commit_info ci;
char hex[GIT_SHA1_HEXSZ + 1];
int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
+ int revision_length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ
: abbrev;
get_commit_info(suspect->commit, &ci, 1);
sha1_to_hex_r(hex, suspect->commit->object.oid.hash);
+ if (opt & OUTPUT_SHOW_TIPS)
+ printf("\t%.*s: %s\n", revision_length, hex, ci.summary.buf);
+
cp = nth_line(sb, ent->lno);
for (cnt = 0; cnt < ent->num_lines; cnt++) {
char ch;
- int length = (opt & OUTPUT_LONG_OBJECT_NAME) ? GIT_SHA1_HEXSZ :
abbrev;
+ int length = revision_length;
if (suspect->commit->object.flags & UNINTERESTING) {
if (blank_boundary)
@@ -2609,6 +2614,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
{ OPTION_CALLBACK, 'C', NULL, &opt, N_("score"), N_("Find line
copies within and across files"), PARSE_OPT_OPTARG, blame_copy_callback },
{ OPTION_CALLBACK, 'M', NULL, &opt, N_("score"), N_("Find line
movements within and across files"), PARSE_OPT_OPTARG, blame_move_callback },
OPT_STRING_LIST('L', NULL, &range_list, N_("n,m"), N_("Process
only line range n,m, counting from 1")),
+ OPT_BIT(0, "tips", &output_option, N_("Show tips before content
lines"), OUTPUT_SHOW_TIPS),
OPT__ABBREV(&abbrev),
OPT_END()
};
--
2.11.0.rc1
Does it make sense to show tips on blame?
Hi!
I'm having fun with difflame. I added support for an option called
'tips' which would display 1-line summary of a block of added lines
that belong to the same revision, in order to provide context while
reading difflame output without having to run an additional git show
command.
Output is like this (from README.txt, taken from difflame on git
itself, sorry if it's too wide):
diff --git a/fast-import.c b/fast-import.c
index f561ba833..64fe602f0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2218,13 +2218,17 @@ static uintmax_t do_change_note_fanout(
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2218)
char *fullpath, unsigned int fullpath_len,
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2219)
unsigned char fanout)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2220) {
-02d0457eb4 (Junio C Hamano 2017-01-10 15:24:26 -0800 2221) struct
tree_content *t = root->tree;
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2221) struct
tree_content *t;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 ) struct
tree_entry *e, leaf;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2223)
unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2224)
uintmax_t num_notes = 0;
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2225)
unsigned char sha1[20];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2226) char
realpath[60];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2227)
405d7f4af fast-import: properly fanout notes when tree is imported
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2228) if (!root->tree)
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2229)
load_tree(root);
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2230) t = root->tree;
+405d7f4af6 (Mike Hommey 2016-12-21 06:04:48 +0900 2231)
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2232) for (i
= 0; t && i < t->entry_count; i++) {
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2233)
e = t->entries[i];
2a113aee9b (Johan Herland 2009-12-07 12:27:24 +0100 2234)
tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
The tip that was used for revision 405d7f4af on both "blocks" of added
lines can be seen.
The question is if you think it makes sense to add this option to
git-blame itself.
Something like:
git blame --tips README.txt
Thanks in advance
difflame
Hi!
For a very long time I had wanted to get the output of diff to include
blame information as well (to see when something was added/removed).
I just created a very small (and rough) tool for that purpose. It's
written in python and if it gets to something better than a small
tool, I think it could be worth to be added into git main (perhaps
into contrib?).
If you want to give ir a try:
https://github.com/eantoranz/difflame
Just provide the two treeishs you would like to diff (no more
parameters are accepted at the time) and you will get the diff along
with blame. Running it right now on the project itself:
✔ ~/difflame [master L|⚑ 1]
23:21 $ ./difflame.py HEAD~3 HEAD~
diff --git a/README.txt b/README.txt
new file mode 100644
index 000..a82aa27
--- /dev/null
+++ b/README.txt
@@ -0,0 +1,11 @@
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 1) difflame
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 2)
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 3)
Copyright 2017 Edmundo Carmona Antoranz
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 4)
Released under the terms of GPLv2
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 5)
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 6) Show
the output of diff with the additional information of blame.
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 7)
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 8)
Lines that remain the same or that were added will indicate when those
lines were 'added' to the file
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 9)
Lines that were removed will display the last revision where those
lines were _present_ on the file (as provided by blame --re
verse)
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 10)
+3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 11) At
the moment, only two parameters need to be provided: two treeishs to
get the diff from
diff --git a/difflame.py b/difflame.py
index f6e879b..06bfc03 100755
--- a/difflame.py
+++ b/difflame.py
@@ -112,16 +112,20 @@ def process_file_from_diff_output(output_lines,
starting_line):
c661286f (Edmundo Carmona Antoranz 2017-01-17 20:10:07 -0600 112)
diff_line = output_lines[i].split()
c661286f (Edmundo Carmona Antoranz 2017-01-17 20:10:07 -0600 113)
if diff_line[0] != "diff":
c661286f (Edmundo Carmona Antoranz 2017-01-17 20:10:07 -0600 114)
raise Exception("Doesn't seem to exist a 'diff' line at line " +
str(i + 1) + ": " + output_lines[i])
-3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 115)
original_name = diff_line[1]
-3d426842 (Edmundo Carmona Antoranz 2017-01-17 22:26:18 -0600 116)
final_name = diff_line[2]
+f135bf04 (Edmundo Carmona Antoranz 2017-01-17 22:50:50 -0600 115)
original_name = diff_line[2]
+f135bf04 (Edmundo Carmona Antoranz 2017-01-17 22:50:50 -0600 116)
final_name = diff_line[3]
c661286f (Edmundo Carmona Antoranz 2017-01-17 20:10:07 -0600 117)
print output_lines[i]; i+=1
.
.
.
Hope you find it useful
Best regards!
Re: Help debugging git-svn
On Fri, Dec 18, 2015 at 11:28 AM, Edmundo Carmona Antoranz wrote: > Ok I came up with another idea to avoid having to deal with the > old svn history (I'm having no problems fetching/dcommitting with my > current repo). I already have the branches I work with, the thing is > that the revisions I fetched before I started using the svn authors > file have nasty IDs instead of human names. I though that I could > create a script to rewrite the whole history of the project adjusting > the usernames to real names (I'll take care of the details, no problem > there... just found out about filter-branch, could work for what I'm > thinking of). My question would be in the direction of rebuilding > git-svn metainfo once I'm done so that I can continue fetching from > svn as if nothing had happened. I checked the documentation in > git-scm.com but didn't find the details about it. Is there a > straight-forward document that explains how the git-svn metadata files > are built? > > Thanks in advance. .rev_map files appear to be simple enough. I'll have fun with them a little bit. Will let you know how it goes later (don't hold your breath it might take a while). -- 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
Forcing git to pack objects
Hi! Recently I was running manually a git gc --prune command (wanted to shrink my 2.8G .git directory by getting rid of loose objects) and I ended up running out of space on my HD. After freaking out a little bit (didn't know if the repo would end up in a 'stable' state), I ended up freeing up some space and I again have a working repo... _but_ I noticed that basically _all_ objects on my repo are laying around in directories .git/objects/00 to ff (and taking a whole lot of space... like the .git directory is now like 5 GBs). After running git gc manually again it ended up taking a lot of time and the objects are still there. Also git svn sometimes gcs after fetching and it took to run cause of the gc execution (ended up killing it) and the files are still there. Is it possible to ask git to put all those objects in .pack files? Or did I mess something on my repo? Just in case, that's a repo I use at work that's working on a windows box (git for windows 2.6.3). Thanks in advance. -- 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: Help debugging git-svn
Ok I came up with another idea to avoid having to deal with the old svn history (I'm having no problems fetching/dcommitting with my current repo). I already have the branches I work with, the thing is that the revisions I fetched before I started using the svn authors file have nasty IDs instead of human names. I though that I could create a script to rewrite the whole history of the project adjusting the usernames to real names (I'll take care of the details, no problem there... just found out about filter-branch, could work for what I'm thinking of). My question would be in the direction of rebuilding git-svn metainfo once I'm done so that I can continue fetching from svn as if nothing had happened. I checked the documentation in git-scm.com but didn't find the details about it. Is there a straight-forward document that explains how the git-svn metadata files are built? Thanks in advance. On Wed, Dec 16, 2015 at 6:36 AM, Edmundo Carmona Antoranz wrote: > On Wed, Dec 16, 2015 at 1:41 AM, Eric Wong wrote: >> >> Any chance you can reproduce this on a Linux system? >> I do not use non-Free systems and have no debugging experience >> there at all. >> > > My wish But it's a big resounding "no". > >>> With my very flawed knowledge of perl I have seen that the process is >>> getting to Ra.pm around here: >> >> It could also be a bug in the SVN bindings or the port of >> Perl. Which versions are you running? > > I'm working on git for windows 2.6.3. I think I could use cygwin, just > in case (I used it before). > >> >> >> I've also been wondering about the motivation of SVN developers to do a >> good job on maintaining their Perl bindings (given git-svn is likely the >> main user of them). >> We've certainly had problems in the past with SVN breaking and >> causing git-svn to segfault around 2011-2012; but it's been a while... -- 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: Help debugging git-svn
On Wed, Dec 16, 2015 at 1:41 AM, Eric Wong wrote: > > Any chance you can reproduce this on a Linux system? > I do not use non-Free systems and have no debugging experience > there at all. > My wish But it's a big resounding "no". >> With my very flawed knowledge of perl I have seen that the process is >> getting to Ra.pm around here: > > It could also be a bug in the SVN bindings or the port of > Perl. Which versions are you running? I'm working on git for windows 2.6.3. I think I could use cygwin, just in case (I used it before). > > > I've also been wondering about the motivation of SVN developers to do a > good job on maintaining their Perl bindings (given git-svn is likely the > main user of them). > We've certainly had problems in the past with SVN breaking and > causing git-svn to segfault around 2011-2012; but it's been a while... -- 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
Help debugging git-svn
Hello, Eric, Everybody!
I need your help getting git-svn to clone a repository.
I had already cloned it once but then a few months ago I discovered
the authors map file and it's like the first time I did a checkout
using git well, perhaps not that much, but close. Seeing the real
names of people when using log, blame, etc is a major difference...
so, back to my tale of sorts.
The thing is that I have already tried to clone it _several_ times and
it always breaks at one point or another (or rather, at one revision
of a branch or another). I have come to think that it's more or less
at the same revisions that it breaks but I'm not 100% certain.
What I _think_ is the cause of most of the problems is that in our
repo people have misplaced branches _inside_ other branches, at least
for a few revisions before realizing their mistake and deleting it.
So say I have a standard layout.
trunk
branches
tags
Then at one point I copy trunk to branches/a
Later on I copy trunk to branches/b
Later on I copy trunk to branches/b/c (instead of branches/c)
And a few revisions later I realize my mistake and copy branches/b/c
to branches/c and remove branches/b/c
I infer this because I'm seeing that on one of the revisions where git
svn usually breaks when fetching has the content of the project inside
a directory, like, say I have directories A, B and C in the project
and I'm seeing that git svn is fetching a revision where the all the
paths of the files are prepended by a directory that looks like a
branch, like this:
branch_name/A/filea.txt
branch_name/A/fileb.txt
etc etc
Instead of
A/filea.txt
A/fileb.txt
So... throw some ideas around that... and then, could you tell a
non-perl developer how to debug it? Perhaps increase verbosity?
One of the errors I see often when fetching (my memory tells me that
it's associated to the branch-in-branch problem but I'm not completely
sure right now), looks like this:
1 [main] perl 5652 cygwin_exception::open_stackdumpfile: Dumping stack
trace to perl.exe.stackdump
And then, in the file:
Exception: STATUS_ACCESS_VIOLATION at rip=0048360C10C
rax=000601E4BFF8 rbx=5219E248 rcx=00060003A590
rdx= rsi=A950 rdi=0004
r8 = r9 = r10=0023
r11=00048D78607A r12=0003 r13=06F54A98
r14=000601E18030 r15=06F54AB0
rbp=00054A88 rsp=0022B810
program=C:\Program Files\Git\usr\bin\perl.exe, pid 5652, thread main
cs=0033 ds=002B es=002B fs=0053 gs=002B ss=002B
With my very flawed knowledge of perl I have seen that the process is
getting to Ra.pm around here:
our $AUTOLOAD;
sub AUTOLOAD {
my $class = ref($_[0]);
$AUTOLOAD =~ s/^${class}::(SUPER::)?//;
return if $AUTOLOAD =~ m/^[A-Z]/;
my $self = shift;
no strict 'refs';
my $method = $self->can("invoke_$AUTOLOAD")
or die "no such method $AUTOLOAD";
no warnings 'uninitialized';
$method->(@$self, @_);
}
The value of $AUTOLOAD there is 'finish_report' but I don't know (or
at least see) where $method->(@$self, @_) is going.
Well... I think that's enough of a mess of a mail (sorry about that).
Hope I was able to provide enough information to at least move a
little bit forward.
Thanks in advance.
--
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
[PATCH v7] blame: add support for --[no-]progress option
--progress can't be used with --incremental or
porcelain formats.
git-annotate inherits the option as well
Helped-by: Eric Sunshine
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 3 ++-
builtin/blame.c | 34 ++
3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..02cb684 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal. Can't use `--progress` together with `--porcelain`
+ or `--incremental`.
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..ba54175 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--progress] [--abbrev=] [ | --contents | --reverse
]
+ [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 2afe828..e78ca09 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
};
+struct progress_info {
+ struct progress *progress;
+ int blamed_lines;
+};
+
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
*suspect, int repeat)
* The blame_entry is found to be guilty for the range.
* Show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+ struct progress_info *pi)
{
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+ pi->blamed_lines += ent->num_lines;
+ display_progress(pi->progress, pi->blamed_lines);
}
/*
@@ -1768,6 +1778,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress_info pi = { NULL, 0 };
+
+ if (show_progress)
+ pi.progress = start_progress_delay(_("Blaming lines"),
+ sb->num_lines, 50, 1);
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1824,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
- found_guilty_entry(ent);
+ found_guilty_entry(ent, &pi);
if (next) {
ent = next;
continue;
@@ -1825,6 +1840,8 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ stop_progress(&pi.progress);
}
static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2537,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for
boundary commits (Default: off)")),
OPT_BOOL(0, "root
Re: [PATCH v6] blame: add support for --[no-]progress option
On Sat, Dec 12, 2015 at 6:37 PM, Eric Sunshine wrote: >> >> Because, if the user didn't provide --[no-]progress option, then the >> value in show_progress will move forward being -1 and then in >> assign_blame, there will be progress output if you chose --incremental >> or porcelain. So, if you chose --incremental or porcelain, we better >> set the value to 0 to make sure there will be _no_ progress. Agree? > > Yeah, I was thinking of that and had the correct interpretation in > mind when reading the code, but then blocked it out of my brain for > some reason when actually composing the response. Good! So, the only things to modify would be: - documentation to reflect new policy - no need to check for show_progress to ask to finish up struct progress instance. Let's give some time to allow for more comments before my next patch version so, say, 5 minutes. -- 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: [PATCH v6] blame: add support for --[no-]progress option
On Sat, Dec 12, 2015 at 6:30 PM, Edmundo Carmona Antoranz
wrote:
>>
>> The 'show_progress = 0' seems unnecessary. What if you did something
>> like this instead?
>>
>> if (show_progress > 0 && (incremental ||
>> (output_option & OUTPUT_PORCELAIN)))
>> die("--progress can't be used with...");
>> else if (show_progress < 0)
>> show_progress = isatty(2);
>>
>>> if (0 < abbrev)
>>> /* one more abbrev length is needed for the boundary commit
>>> */
>>> abbrev++;
>
> Because, if the user didn't provide --[no-]progress option, then the
> value in show_progress will move forward being -1 and then in
> assign_blame, there will be progress output if you chose --incremental
> or porcelain. So, if you chose --incremental or porcelain, we better
> set the value to 0 to make sure there will be _no_ progress. Agree?
H if the code in assign_blame changed to this, it would be
possible to allow the -1 to go through:
if (show_progress > 0)
pi.progress = start_progress_delay(_("Blaming lines"),
sb->num_lines, 50, 1);
But then I think it would be more 'concise' if we had the value set to
0/1 instead of expecting to see a possible value of -1 there (or
anywhere else) after progressing if progress will be shown or not in
the piece of code we are chatting about.
Comments?
--
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: [PATCH v6] blame: add support for --[no-]progress option
On Sat, Dec 12, 2015 at 6:17 PM, Eric Sunshine wrote:
> Right here below the "---" line would be a good place to explain what
> changed since the previous version. As an aid for reviewers, it's also
> helpful to provide a link to the previous round, like this[1].
>
> [1]: http://thread.gmane.org/gmane.comp.version-control.git/281677
>
Ok... learning the tricks.
>> @@ -69,6 +69,13 @@ include::line-range-format.txt[]
>> +--[no-]progress::
>> + Progress status is reported on the standard error stream
>> + by default when it is attached to a terminal. This flag
>> + enables progress reporting even if not attached to a
>> + terminal. Progress information won't be displayed if using
>> + `--porcelain` or `--incremental`.
>
> The actual implementation (below) actively forbids --progress with
> --porcelain or --incremental, so the final sentence is misleading.
> Perhaps say instead that "--progress is incompatible with --porcelain
> and --incremental".
>
> More below...
>
Absolutely right didn't reflect the 'policy change' in the
documentation. Will update for next patch version.
>> +
>> + if (pi.progress)
>> + stop_progress(&pi.progress);
>
> As noted in the v5 review[2], stop_progress() itself handles NULL
> 'struct progress' gracefully, so the 'if (pi.progress)' conditional is
> unnecessary, thus the code can be simplified further to merely:
>
> stop_progress(&pi.progress);
>
You are right!
>
> The 'show_progress = 0' seems unnecessary. What if you did something
> like this instead?
>
> if (show_progress > 0 && (incremental ||
> (output_option & OUTPUT_PORCELAIN)))
> die("--progress can't be used with...");
> else if (show_progress < 0)
> show_progress = isatty(2);
>
>> if (0 < abbrev)
>> /* one more abbrev length is needed for the boundary commit
>> */
>> abbrev++;
Because, if the user didn't provide --[no-]progress option, then the
value in show_progress will move forward being -1 and then in
assign_blame, there will be progress output if you chose --incremental
or porcelain. So, if you chose --incremental or porcelain, we better
set the value to 0 to make sure there will be _no_ progress. Agree?
Cheers!
--
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: [PATCH v6] blame: add support for --[no-]progress option
On Sat, Dec 12, 2015 at 5:57 PM, Edmundo Carmona Antoranz
wrote:
> + if (incremental || (output_option & OUTPUT_PORCELAIN)) {
> + if (show_progress > 0)
> + die("--progress can't be used with --incremental or
> porcelain formats");
> + show_progress = 0;
> + } else if (show_progress < 0)
> + show_progress = isatty(2);
> +
I think I took care of all the comments from Eric gracefully. Hope
that block is ok for detection of mutually exclusivity related to
--progress.
Cheers!
--
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
[PATCH v6] blame: add support for --[no-]progress option
--progress can't be used with --incremental or
porcelain formats.
git-annotate inherits the option as well
Helped-by: Eric Sunshine
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 3 ++-
builtin/blame.c | 35 +++
3 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..ef642b9 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal. Progress information won't be displayed if using
+ `--porcelain` or `--incremental`.
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..ba54175 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--progress] [--abbrev=] [ | --contents | --reverse
]
+ [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 2afe828..be5d73d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
};
+struct progress_info {
+ struct progress *progress;
+ int blamed_lines;
+};
+
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
*suspect, int repeat)
* The blame_entry is found to be guilty for the range.
* Show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+ struct progress_info *pi)
{
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+ pi->blamed_lines += ent->num_lines;
+ display_progress(pi->progress, pi->blamed_lines);
}
/*
@@ -1768,6 +1778,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress_info pi = { NULL, 0 };
+
+ if (show_progress)
+ pi.progress = start_progress_delay(_("Blaming lines"),
+ sb->num_lines, 50, 1);
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1824,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
- found_guilty_entry(ent);
+ found_guilty_entry(ent, &pi);
if (next) {
ent = next;
continue;
@@ -1825,6 +1840,9 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (pi.progress)
+ stop_progress(&pi.progress);
}
static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2538,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for
boundary commits (Default: off)&q
Re: [PATCH v5] blame: add support for --[no-]progress option
Hey, Eric!
On Tue, Dec 8, 2015 at 2:22 AM, Eric Sunshine wrote:
> On Tue, Nov 24, 2015 at 11:36 PM, Edmundo Carmona Antoranz
> wrote:
>> * created struct progress_info in builtin/blame.c
>> this struct holds the information used to display progress so
>> that only one additional parameter is passed to
>> found_guilty_entry().
>
> Commit messages typically are composed of proper sentences and
> paragraphs rather than bullets points. Also, write in imperative mood:
> "Create progress_info..."
>
> In this case, though, the information in this bullet point isn't all
> that interesting for the commit message since it's a detail of
> implementation easily gleaned by reading the patch itself. There's
> nothing particularly tricky about it, thus it doesn't really deserve
> to be called out as special in the commit message.
>
> What might be more interesting and deserve mention in the commit
> message is how this new option interacts with porcelain and
> incremental modes and why the choice was made.
>
> More below...
Ok
>
>> * --[no-]progress option is also inherited by git-annotate.
>>
>> Signed-off-by: Edmundo Carmona Antoranz
>> ---
>> diff --git a/Documentation/blame-options.txt
>> b/Documentation/blame-options.txt
>> @@ -69,6 +69,13 @@ include::line-range-format.txt[]
>> iso format is used. For supported values, see the discussion
>> of the --date option at linkgit:git-log[1].
>>
>> +--[no-]progress::
>> + Progress status is reported on the standard error stream
>> + by default when it is attached to a terminal. This flag
>> + enables progress reporting even if not attached to a
>> + terminal. Progress information won't be displayed if using
>> + `--porcelain` or `--incremental`.
>
> Is silently ignoring --progress a good idea when combined with
> --porcelain or --incremental, or would it be better to error out with
> a "mutually exclusive options" diagnostic? (Genuine question.)
>
I think it makes sense (and so it's documented so people can know what
could be going on but detecting mutually exclusive options and
erroring out could also make sense. Who could give some insight?
>> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
>> @@ -10,7 +10,8 @@ SYNOPSIS
>> [verse]
>> 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
>> [--incremental]
>> [-L ] [-S ] [-M] [-C] [-C] [-C]
>> [--since=]
>> - [--abbrev=] [ | --contents | --reverse ]
>> [--]
>> + [--[no-]progress] [--abbrev=] [ | --contents |
>> --reverse ]
>
> Hmm, is [--[no-]progress] common in Git documentation? (Genuine
> question.) For the synopsis, I'd have expected to see only
> [--progress] and leave it to the more detailed description of the
> option to mention the possibility of negation (but I haven't
> double-checked other documentation, so my expectation may be skewed).
>
Hmmm hadn't noticed that. Will remove it from this part.
>> DESCRIPTION
>> ---
>> diff --git a/builtin/blame.c b/builtin/blame.c
>> @@ -127,6 +129,11 @@ struct origin {
>> +struct progress_info {
>> + struct progress *progress;
>> + int blamed_lines;
>> +};
>> @@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
>> *suspect, int repeat)
>> -static void found_guilty_entry(struct blame_entry *ent)
>> +static void found_guilty_entry(struct blame_entry *ent,
>> + struct progress_info *pi)
>> @@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
>> + if (pi)
>> + display_progress(pi->progress, pi->blamed_lines +=
>> ent->num_lines);
>
> This is updating of 'blamed_lines' is rather subtle and easily
> overlooked. Splitting it out as a separate statement could improve
> readability:
>
> pi->blamed_lines += ent->num_lines;
> display_progress(pi->progress, pi->blamed_lines);
>
Ok.
>> }
>> @@ -1768,6 +1778,16 @@ static void assign_blame(struct scoreboard *sb, int
>> opt)
>> {
>> struct rev_info *revs = sb->revs;
>> struct commit *commit = prio_queue_get(&sb->commits);
>> + struct progress_info *pi = NULL;
>> +
>> + if (show_progress) {
>> + pi = malloc(sizeof(*pi));
>> + if (pi) {
>
> xmalloc(), unlike malloc(), will die() upon allocation failure which
> would allow you to avoid
Re: What's cooking in git.git (Dec 2015, #02; Fri, 4)
Hi, Junio, Jeff! On Fri, Dec 4, 2015 at 5:15 PM, Junio C Hamano wrote: > * ec/annotate-deleted (2015-11-20) 1 commit > - annotate: skip checking working tree if a revision is provided > > Usability fix for annotate-specific " " syntax with deleted > files. > > Waiting for review. Is there something I have to do about it? And, another thing, I had sent the fifth version of my patch about progress for blame where I took care of all the comments hat Junio stated before and I got no comments since. I hoped that patch would be listed sometime but I didn't see it in this what's cooking or the previous one. Maybe it's bellow your radar? Is there something else I can to do about it? ($gmane/281677) Thanks in advance! -- 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
[PATCH v5] blame: add support for --[no-]progress option
* created struct progress_info in builtin/blame.c
this struct holds the information used to display progress so
that only one additional parameter is passed to
found_guilty_entry().
* --[no-]progress option is also inherited by git-annotate.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 3 ++-
builtin/blame.c | 40
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..ef642b9 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal. Progress information won't be displayed if using
+ `--porcelain` or `--incremental`.
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..b0154c2 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--[no-]progress] [--abbrev=] [ | --contents |
--reverse ]
+ [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..954d32c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
};
+struct progress_info {
+ struct progress *progress;
+ int blamed_lines;
+};
+
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
*suspect, int repeat)
* The blame_entry is found to be guilty for the range.
* Show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+ struct progress_info *pi)
{
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+ if (pi)
+ display_progress(pi->progress, pi->blamed_lines +=
ent->num_lines);
}
/*
@@ -1768,6 +1778,16 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress_info *pi = NULL;
+
+ if (show_progress) {
+ pi = malloc(sizeof(*pi));
+ if (pi) {
+ pi->progress = start_progress_delay(_("Blaming lines"),
+ sb->num_lines, 50,
1);
+ pi->blamed_lines = 0;
+ }
+ }
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1829,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
- found_guilty_entry(ent);
+ found_guilty_entry(ent, pi);
if (next) {
ent = next;
continue;
@@ -1825,6 +1845,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (pi) {
+ stop_progress(&pi->progress);
+ free(pi);
Re: [PATCH v4] blame: add support for --[no-]progress option
On Mon, Nov 23, 2015 at 11:57 PM, Torsten Bögershausen wrote: > Minor remark: The '*' should be close to the variable: "struct progress_info > *pi" Nice catch, Torsten. I had already caught it as the standard... that one slipped by. Was waiting for more comments to show up, but none so far so I'll send another version with this adjustment. Thanks again! -- 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: [PATCH v4] blame: add support for --[no-]progress option
On Mon, Nov 23, 2015 at 7:07 PM, Edmundo Carmona Antoranz
wrote:
> {
> struct rev_info *revs = sb->revs;
> struct commit *commit = prio_queue_get(&sb->commits);
> + struct progress_info *pi = NULL;
Switched to using pointer to make it more elegant.
--
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
[PATCH v4] blame: add support for --[no-]progress option
* created struct progress_info in builtin/blame.c
this struct holds the information used to display progress so
that only one additional parameter is passed to
found_guilty_entry().
* --[no-]progress option is also inherited by git-annotate.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 3 ++-
builtin/blame.c | 40
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..ef642b9 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal. Progress information won't be displayed if using
+ `--porcelain` or `--incremental`.
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..b0154c2 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--[no-]progress] [--abbrev=] [ | --contents |
--reverse ]
+ [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..1d43b52 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
};
+struct progress_info {
+ struct progress *progress;
+ int blamed_lines;
+};
+
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
*suspect, int repeat)
* The blame_entry is found to be guilty for the range.
* Show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+ struct progress_info * pi)
{
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,8 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+ if (pi)
+ display_progress(pi->progress, pi->blamed_lines +=
ent->num_lines);
}
/*
@@ -1768,6 +1778,16 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress_info *pi = NULL;
+
+ if (show_progress) {
+ pi = malloc(sizeof(*pi));
+ if (pi) {
+ pi->progress = start_progress_delay(_("Blaming lines"),
+ sb->num_lines, 50,
1);
+ pi->blamed_lines = 0;
+ }
+ }
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1829,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
- found_guilty_entry(ent);
+ found_guilty_entry(ent, pi);
if (next) {
ent = next;
continue;
@@ -1825,6 +1845,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (pi) {
+ stop_progress(&pi->progress);
+ free(pi);
Re: [PATCH v3] blame: add support for --[no-]progress option
On Sun, Nov 22, 2015 at 11:12 PM, Edmundo Carmona Antoranz
wrote:
> +struct progress_info {
> + struct progress *progress;
> + int blamed_lines;
> +};
> +
This is valid, right? Adding 2 more parameters to found_guilty_entry()
sounded like an overkill so I joined them into a new struct.
Thanks in advance.
--
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
[PATCH v3] blame: add support for --[no-]progress option
* created struct progress_info in builtin/blame.c
this struct holds the information used to display progress so
that only one additional parameter is passed to
found_guilty_entry().
* --[no-]progress option is also inherited by git-annotate.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 3 ++-
builtin/blame.c | 37 +
3 files changed, 42 insertions(+), 5 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..ef642b9 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal. Progress information won't be displayed if using
+ `--porcelain` or `--incremental`.
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..b0154c2 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--[no-]progress] [--abbrev=] [ | --contents |
--reverse ]
+ [--]
DESCRIPTION
---
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..374ea7c 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -127,6 +129,11 @@ struct origin {
char path[FLEX_ARRAY];
};
+struct progress_info {
+ struct progress *progress;
+ int blamed_lines;
+};
+
static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)
{
@@ -1746,7 +1753,8 @@ static int emit_one_suspect_detail(struct origin
*suspect, int repeat)
* The blame_entry is found to be guilty for the range.
* Show it in incremental output.
*/
-static void found_guilty_entry(struct blame_entry *ent)
+static void found_guilty_entry(struct blame_entry *ent,
+ struct progress_info * pi)
{
if (incremental) {
struct origin *suspect = ent->suspect;
@@ -1758,6 +1766,10 @@ static void found_guilty_entry(struct blame_entry *ent)
write_filename_info(suspect->path);
maybe_flush_or_die(stdout, "stdout");
}
+ if (show_progress) {
+ pi->blamed_lines += ent->num_lines;
+ display_progress(pi->progress, pi->blamed_lines);
+ }
}
/*
@@ -1768,6 +1780,13 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress_info pi;
+
+ if (show_progress) {
+ pi.progress = start_progress_delay(_("Blaming lines"),
+ sb->num_lines, 50, 1);
+ pi.blamed_lines = 0;
+ }
while (commit) {
struct blame_entry *ent;
@@ -1809,7 +1828,7 @@ static void assign_blame(struct scoreboard *sb, int opt)
suspect->guilty = 1;
for (;;) {
struct blame_entry *next = ent->next;
- found_guilty_entry(ent);
+ found_guilty_entry(ent, &pi);
if (next) {
ent = next;
continue;
@@ -1825,6 +1844,9 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (show_progress)
+ stop_progress(&pi.progress);
}
static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6
Re: [PATCH v2] blame: add support for --[no-]progress option
Nice to see you back, Junio!
On Sun, Nov 22, 2015 at 8:08 PM, Junio C Hamano wrote:
> Edmundo Carmona Antoranz writes:
>
>> Will also affect annotate
>
> Is that a good thing? In any case, make it understandable without
> the title line (i.e. make it a full sentence, ending with a full
> stop).
>
Will make the explanation a little more verbose. About being a bad
thing, I don't see how, it's just the same functionality. You think I
should turn it off if using annotate?
>> + if (progress) {
>> + for (next = suspect->suspects; next !=
>> NULL;
>> + next = next->next)
>> + blamed_lines +=
>> next->num_lines;
>> + display_progress(progress,
>> blamed_lines);
>> + }
>
> Is this math and the placement of the code correct? It would
> probably be more obvious if this hunk is in found_guilty_entry(),
> which is already the dedicated function in which we report about a
> group of lines whose ultimate origin has become clear.
>
I'll see what I can do about it.
>
> Two comments.
>
> * How does this interact with incremental or porcelain blame?
>Shouldn't progress be turned off when these modes are in use?
Given that they are supposed to be for machine consumption, I'll turn
progress off is using one of them.
>
> * Shouldn't progress be turned off if the result comes very
>quickly, using start_progress_delay()?
>
Ok. Default values as in checkout? 50, 1?
--
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: [PATCH v2] blame: add support for --[no-]progress option
Hey, Johannes! An oversight! git-annotate.txt took me to blame-options.txt and I didn't notice it's also pointed to from git-blame.txt. Let's wait for some comments about the coding (or blessings) so that I can send another patch. Thanks! On Sun, Nov 22, 2015 at 1:58 PM, Johannes Sixt wrote: > Am 22.11.2015 um 17:02 schrieb Edmundo Carmona Antoranz: >> >> Will also affect annotate >> >> Signed-off-by: Edmundo Carmona Antoranz >> --- >> Documentation/blame-options.txt | 7 +++ >> Documentation/git-blame.txt | 9 - >> builtin/blame.c | 25 +++-- >> 3 files changed, 38 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/blame-options.txt >> b/Documentation/blame-options.txt >> index 760eab7..43f4f08 100644 >> --- a/Documentation/blame-options.txt >> +++ b/Documentation/blame-options.txt >> @@ -69,6 +69,13 @@ include::line-range-format.txt[] >> iso format is used. For supported values, see the discussion >> of the --date option at linkgit:git-log[1]. >> >> +--[no-]progress:: >> + Progress status is reported on the standard error stream >> + by default when it is attached to a terminal. This flag >> + enables progress reporting even if not attached to a >> + terminal. >> + >> + >> -M||:: >> Detect moved or copied lines within a file. When a commit >> moves or copies a block of lines (e.g. the original file >> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt >> index e6e947c..2e63397 100644 >> --- a/Documentation/git-blame.txt >> +++ b/Documentation/git-blame.txt >> @@ -10,7 +10,8 @@ SYNOPSIS >> [verse] >> 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w] >> [--incremental] >> [-L ] [-S ] [-M] [-C] [-C] [-C] >> [--since=] >> - [--abbrev=] [ | --contents | --reverse ] >> [--] >> + [--[no-]progress] [--abbrev=] [ | --contents | >> --reverse ] >> + [--] > > > You add the option to to the synopsis of git-blame.txt, but not to > git-annotate.txt. > >> >> DESCRIPTION >> --- >> @@ -88,6 +89,12 @@ include::blame-options.txt[] >> abbreviated object name, use +1 digits. Note that 1 column >> is used for a caret to mark the boundary commit. >> >> +--[no-]progress:: >> + Progress status is reported on the standard error stream >> + by default when it is attached to a terminal. This flag >> + enables progress reporting even if not attached to a >> + terminal. >> + > > > Any particular reason you add this text twice? As can be seen on the hunk > header, git-blame.txt includes blame-options.txt. > > -- Hannes > -- 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
[PATCH v2] blame: add support for --[no-]progress option
Will also affect annotate
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 9 -
builtin/blame.c | 25 +++--
3 files changed, 38 insertions(+), 3 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..43f4f08 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal.
+
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..2e63397 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--[no-]progress] [--abbrev=] [ | --contents |
--reverse ]
+ [--]
DESCRIPTION
---
@@ -88,6 +89,12 @@ include::blame-options.txt[]
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal.
+
THE PORCELAIN FORMAT
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..480bb2d 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -1768,6 +1770,11 @@ static void assign_blame(struct scoreboard *sb, int opt)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress * progress = NULL;
+ int blamed_lines = 0;
+
+ if (show_progress)
+ progress = start_progress(_("Blaming lines"), sb->num_lines);
while (commit) {
struct blame_entry *ent;
@@ -1814,6 +1821,12 @@ static void assign_blame(struct scoreboard *sb, int opt)
ent = next;
continue;
}
+ if (progress) {
+ for (next = suspect->suspects; next !=
NULL;
+next = next->next)
+ blamed_lines += next->num_lines;
+ display_progress(progress,
blamed_lines);
+ }
ent->next = sb->ent;
sb->ent = suspect->suspects;
suspect->suspects = NULL;
@@ -1825,6 +1838,9 @@ static void assign_blame(struct scoreboard *sb, int opt)
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (progress)
+ stop_progress(&progress);
}
static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2536,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for
boundary commits (Default: off)")),
OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits
as boundaries (Default: off)")),
OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost
statistics")),
+ OPT_BOOL(0, "progress", &show_progress, N_("Force progress
reporting")),
OPT_BIT(0, "score-debug", &output_option, N_("Show output score
for blame entries"), OUTPUT_SHOW_SCORE),
OPT_BIT('f', "show-name", &output_opti
Re: [PATCH v1] blame: add support for --[no-]progress option
On Sat, Nov 21, 2015 at 11:27 PM, Edmundo Carmona Antoranz wrote: > -static void assign_blame(struct scoreboard *sb, int opt) > +static void assign_blame(struct scoreboard *sb, int opt, int show_progress) > > Would it be better to include show_progress as a binary flag in opt > instead of a separate variable? What is the point of having 'global' variable and not using it as such? I'm sending a second version of the patch where I'm taking care of most of my previous comments. Let's hope that one holds water. Thanks in advance. -- 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: [PATCH v1] blame: add support for --[no-]progress option
I think I found where to calculate the number of blamed lines without
having to do it from scratch every cycle. It's where the list of
sb->ent is getting its nodes pointed around here:
for (;;) {
struct blame_entry *next = ent->next;
found_guilty_entry(ent);
if (next) {
ent = next;
continue;
}
ent->next = sb->ent;
sb->ent = suspect->suspects;
suspect->suspects = NULL;
break;
}
So the function I added to blame.c, it's gone.
--
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: [PATCH v1] blame: add support for --[no-]progress option
Hey, guys!
Time for some more patch destruction.
On Sat, Nov 21, 2015 at 11:11 PM, Edmundo Carmona Antoranz
wrote:
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct scoreboard *sb, int opt, int show_progress)
Would it be better to include show_progress as a binary flag in opt
instead of a separate variable?
> + struct progress * progress = NULL;
> + int blamed_lines = -1;
I'm wondering if it would be better to save the 'last number used in
progress' inside struct progress so that we could skip blamed_lines.
That would also allow progress itself (as in the API) to avoid
printing duplicated progress values. In my case, I'm going through a
number of cycles where _I think_ I might have the same number of
blamed lines detected. To avoid asking to do a new progress print out
with the same value, I'm checking the value I had used last time. If
progress took care of that check up, this variable would go for good
and I would just ask progress to print with the current value (no
matter if it's duplicate or an increased value).
> +
> + if (show_progress) {
> + progress = start_progress(_("Blaming lines"), sb->num_lines);
> + }
Already noticed I can get rid of the curly brackets.
>
> while (commit) {
> struct blame_entry *ent;
> @@ -1822,9 +1838,21 @@ static void assign_blame(struct scoreboard *sb, int
> opt)
> }
> origin_decref(suspect);
>
> + if (progress) {
> + int current_blamed_lines = count_blamed_lines(sb);
> + if (current_blamed_lines > blamed_lines) {
> + blamed_lines = current_blamed_lines;
> + display_progress(progress, blamed_lines);
> + }
> + }
> +
> if (DEBUG) /* sanity */
> sanity_check_refcnt(sb);
> }
> +
> + if (progress) {
> + stop_progress(&progress);
> + }
> }
>
It first I tried to detect how many revisions where going to be
checked so that I could report progress on them but I found extracting
information from scoreboard a little tricky (to be read: I could not
extract anything out of it). Then I though of counting lines and it
works so (same thing with the curly brackets).
> + assign_blame(&sb, opt, show_progress);
> +
> if (!incremental)
> setup_pager();
>
> - assign_blame(&sb, opt);
> -
setup_pager() was breaking progress so I moved it _after_
assign_blame() and it seems to behave. Hope that's not a problem.
Looking forward to your feedback.
--
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
[PATCH v1] blame: add support for --[no-]progress option
Will also affect annotate
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/blame-options.txt | 7 +++
Documentation/git-blame.txt | 9 -
builtin/blame.c | 39 ---
3 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 760eab7..43f4f08 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -69,6 +69,13 @@ include::line-range-format.txt[]
iso format is used. For supported values, see the discussion
of the --date option at linkgit:git-log[1].
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal.
+
+
-M||::
Detect moved or copied lines within a file. When a commit
moves or copies a block of lines (e.g. the original file
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index e6e947c..2e63397 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -10,7 +10,8 @@ SYNOPSIS
[verse]
'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-e] [-p] [-w]
[--incremental]
[-L ] [-S ] [-M] [-C] [-C] [-C] [--since=]
- [--abbrev=] [ | --contents | --reverse ] [--]
+ [--[no-]progress] [--abbrev=] [ | --contents |
--reverse ]
+ [--]
DESCRIPTION
---
@@ -88,6 +89,12 @@ include::blame-options.txt[]
abbreviated object name, use +1 digits. Note that 1 column
is used for a caret to mark the boundary commit.
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal. This flag
+ enables progress reporting even if not attached to a
+ terminal.
+
THE PORCELAIN FORMAT
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..31477d8 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -28,6 +28,7 @@
#include "line-range.h"
#include "line-log.h"
#include "dir.h"
+#include "progress.h"
static char blame_usage[] = N_("git blame [] [] []
[--] ");
@@ -50,6 +51,7 @@ static int incremental;
static int xdl_opts;
static int abbrev = -1;
static int no_whole_file_rename;
+static int show_progress;
static struct date_mode blame_date_mode = { DATE_ISO8601 };
static size_t blame_date_width;
@@ -1760,14 +1762,28 @@ static void found_guilty_entry(struct blame_entry *ent)
}
}
+static int count_blamed_lines(struct scoreboard *sb)
+{
+ int counter = 0;
+ for (struct blame_entry * entry = sb->ent; entry; entry = entry->next)
+ counter += entry->num_lines;
+ return counter;
+}
+
/*
* The main loop -- while we have blobs with lines whose true origin
* is still unknown, pick one blob, and allow its lines to pass blames
* to its parents. */
-static void assign_blame(struct scoreboard *sb, int opt)
+static void assign_blame(struct scoreboard *sb, int opt, int show_progress)
{
struct rev_info *revs = sb->revs;
struct commit *commit = prio_queue_get(&sb->commits);
+ struct progress * progress = NULL;
+ int blamed_lines = -1;
+
+ if (show_progress) {
+ progress = start_progress(_("Blaming lines"), sb->num_lines);
+ }
while (commit) {
struct blame_entry *ent;
@@ -1822,9 +1838,21 @@ static void assign_blame(struct scoreboard *sb, int opt)
}
origin_decref(suspect);
+ if (progress) {
+ int current_blamed_lines = count_blamed_lines(sb);
+ if (current_blamed_lines > blamed_lines) {
+ blamed_lines = current_blamed_lines;
+ display_progress(progress, blamed_lines);
+ }
+ }
+
if (DEBUG) /* sanity */
sanity_check_refcnt(sb);
}
+
+ if (progress) {
+ stop_progress(&progress);
+ }
}
static const char *format_time(unsigned long time, const char *tz_str,
@@ -2520,6 +2548,7 @@ int cmd_blame(int argc, const char **argv, const char
*prefix)
OPT_BOOL('b', NULL, &blank_boundary, N_("Show blank SHA-1 for
boundary commits (Default: off)")),
OPT_BOOL(0, "root", &show_root, N_("Do not treat root commits
as boundaries (Default: off)")),
OPT_BOOL(0, "show-stats", &show_stats, N_("Show work cost
statistics")),
+ OPT_BOOL(0, "progress", &show_progress, N_("Force progress
reporting")),
Re: [PATCH v1] annotate: skip checking working tree if a revision is provided
And I did't say it right. The execution path where dashdash_pos is 0
is coming from "annotate". Sorry for my confusion on the previous
mail.
On Fri, Nov 20, 2015 at 7:25 PM, Edmundo Carmona Antoranz
wrote:
> On Tue, Nov 17, 2015 at 7:20 PM, Edmundo Carmona Antoranz
> wrote:
>> + if (!revs.pending.nr && !file_exists(path))
>> + die_errno("cannot stat path '%s'", path);
>> +
>
> I was wondering if I should only check the path that is coming from
> "blame" (which is where I'm taking the check from) by checking
> dashdash_pos:
>
> if (!dashdash_pos && !revs.pending.nr && !file_exists(path))
> die_errno("cannot stat path '%s'", path);
>
> So that we don't apply the check if we are coming from normal
> dashdash_pos != 0 (and which didn't apply the filesystem check).
>
> Thanks in advance.
--
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: [PATCH v1] annotate: skip checking working tree if a revision is provided
On Tue, Nov 17, 2015 at 7:20 PM, Edmundo Carmona Antoranz
wrote:
> + if (!revs.pending.nr && !file_exists(path))
> + die_errno("cannot stat path '%s'", path);
> +
I was wondering if I should only check the path that is coming from
"blame" (which is where I'm taking the check from) by checking
dashdash_pos:
if (!dashdash_pos && !revs.pending.nr && !file_exists(path))
die_errno("cannot stat path '%s'", path);
So that we don't apply the check if we are coming from normal
dashdash_pos != 0 (and which didn't apply the filesystem check).
Thanks in advance.
--
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
[PATCH v1] annotate: skip checking working tree if a revision is provided
If a file has been deleted/renamed, annotate refuses to work
because the file does not exist on the working tree anymore
(even if the path provided does match a blob on said revision).
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/blame.c | 5 +++--
t/annotate-tests.sh | 20
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..856971a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,12 +2683,13 @@ parse_done:
argv[argc - 1] = "--";
setup_work_tree();
- if (!file_exists(path))
- die_errno("cannot stat path '%s'", path);
}
revs.disable_stdin = 1;
setup_revisions(argc, argv, &revs, NULL);
+ if (!revs.pending.nr && !file_exists(path))
+ die_errno("cannot stat path '%s'", path);
+
memset(&sb, 0, sizeof(sb));
sb.revs = &revs;
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index b1673b3..c99ec41 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -567,3 +567,23 @@ test_expect_success 'blame -L X,-N (non-numeric N)' '
test_expect_success 'blame -L ,^/RE/' '
test_must_fail $PROG -L1,^/99/ file
'
+
+test_expect_success 'annotate deleted file' '
+ echo hello world > hello_world.txt &&
+ git add hello_world.txt &&
+ git commit -m "step 1" &&
+ git rm hello_world.txt &&
+ git commit -m "step 2" &&
+ git annotate hello_world.txt HEAD~1 &&
+ test_must_fail git annotate hello_world.txt
+'
+
+test_expect_success 'annotate moved file' '
+ echo hello world > hello_world.txt &&
+ git add hello_world.txt &&
+ git commit -m "step 1" &&
+ git mv hello_world.txt not_there_anymore.txt &&
+ git commit -m "step 2" &&
+ git annotate hello_world.txt HEAD~1 &&
+ test_must_fail git annotate hello_world.txt
+'
--
2.6.2
--
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: [PATCH v2] blame: avoid checking if a file exists on the working tree if a revision is provided
On Tue, Nov 17, 2015 at 5:37 PM, Edmundo Carmona Antoranz wrote: > So, do I forget about the blame patch (given that I'm not fixing an > advertised syntax, even if it's supported) and fix annotate instead or > do I fix both? And if I should swing for both, do I create a single > patch or a chain of two patches, one for each builtin? Actually, cmd_annotate will call cmd_blame so this patch actually fixes annotate as well (nice unintended consequence). So I guess it will be a single patch. Let me work on the tests and then I'll send a patch that will hopefully cover all raised points. Cheers! -- 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: [PATCH v2] blame: avoid checking if a file exists on the working tree if a revision is provided
On Mon, Nov 16, 2015 at 11:11 PM, Eric Sunshine wrote: > > This subject is a bit long; try to keep it to about 72 characters or less. > > More importantly, though, it doesn't give us a high level overview of > the purpose of the patch, which is that it is fixing blame to work on > a file at a given revision even if the file no longer exists in the > worktree. Sure! > git-blame documentation does not advertise "blame " as a > valid invocation. It does advertise "blame -- ", and this > case already works correctly even when does not exist in the > worktree. > > git-annotate documentation, on the other hand, does advertise > "annotate ", and it fails to work when is absent > from the worktree, so perhaps you want to sell this patch as fixing > git-annotate instead? So, do I forget about the blame patch (given that I'm not fixing an advertised syntax, even if it's supported) and fix annotate instead or do I fix both? And if I should swing for both, do I create a single patch or a chain of two patches, one for each builtin? > This example is certainly illustrative, but an even more common case > may be trying to blame/annotate a file which existed in an older > revision but doesn't exist anymore at HEAD, thus is absent from the > worktree. Such a case could likely be described in a sentence or two > without resorting to verbose examples (though, not a big deal if you > keep the example). K. > > A new test or two would be welcome, possibly in t/annotate-tests.sh if > you consider this also fixing git-blame, or in t8001-annotate.sh if > you're selling it only as a fix for git-annotate. I guess I'll wait for feedback about my first question before I decide what I will do about the tests. Thank you very much for your comments, Eric. -- 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
[PATCH v2] blame: avoid checking if a file exists on the working tree if a revision is provided
If a file has been deleted/renamed, blame refused to display
blame content even if the path provided does match an existing
blob on said revision.
$ git status
On branch hide
Changes not staged for commit:
(use "git add/rm ..." to update what will be committed)
(use "git checkout -- ..." to discard changes in working directory)
deleted:testfile1.txt
no changes added to commit (use "git add" and/or "git commit -a")
Before:
$ git blame testfile1.txt
fatal: cannot stat path 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
fatal: cannot stat path 'testfile1.txt': No such file or directory
After:
$ git blame testfile1.txt
fatal: Cannot lstat 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1)
testfile 2
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3)
Some content
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/blame.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..856971a 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,12 +2683,13 @@ parse_done:
argv[argc - 1] = "--";
setup_work_tree();
- if (!file_exists(path))
- die_errno("cannot stat path '%s'", path);
}
revs.disable_stdin = 1;
setup_revisions(argc, argv, &revs, NULL);
+ if (!revs.pending.nr && !file_exists(path))
+ die_errno("cannot stat path '%s'", path);
+
memset(&sb, 0, sizeof(sb));
sb.revs = &revs;
--
2.6.2
--
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: [PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided
Ok... I think I got how to check revisions / file existence after
revisions have been set up.
Sending next patch version
On Mon, Nov 16, 2015 at 7:16 PM, Edmundo Carmona Antoranz
wrote:
> On Mon, Nov 16, 2015 at 7:07 PM, Edmundo Carmona Antoranz
> wrote:
>> - if (!file_exists(path))
>> - die_errno("cannot stat path '%s'", path);
>
> Two things:
> 1
> What I would love to do is check if revisions were provided. Something like:
>
> if (no_revisions() && !file_exists(path))
> die_errno("cannot stat path '%s'", path);
>
> _but_ revisions are set up a little bit later. I don't know right now
> if I could just move it up (I don't think it would be that simple
> because I see there's some messing up with argv and argc in that 'if'
> that encloses the lines I'm removing). Maybe it would make sense to
> move the check for file existence to _after_ revisions have been set
> up? Even without the check for revisions, it's behaving kind of the
> way I mean it to. But I'm sure it'd be more elegant if I checked if
> revisions were provided.
>
> 2 It makes sense to create test cases for this patch, right?
>
> Looking forward to your comments. Thanks in advance.
--
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: [PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided
On Mon, Nov 16, 2015 at 7:07 PM, Edmundo Carmona Antoranz
wrote:
> - if (!file_exists(path))
> - die_errno("cannot stat path '%s'", path);
Two things:
1
What I would love to do is check if revisions were provided. Something like:
if (no_revisions() && !file_exists(path))
die_errno("cannot stat path '%s'", path);
_but_ revisions are set up a little bit later. I don't know right now
if I could just move it up (I don't think it would be that simple
because I see there's some messing up with argv and argc in that 'if'
that encloses the lines I'm removing). Maybe it would make sense to
move the check for file existence to _after_ revisions have been set
up? Even without the check for revisions, it's behaving kind of the
way I mean it to. But I'm sure it'd be more elegant if I checked if
revisions were provided.
2 It makes sense to create test cases for this patch, right?
Looking forward to your comments. Thanks in advance.
--
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
[PATCH v1] blame: avoid checking if a file exists on the working tree if a revision is provided
If a file has been deleted/renamed, blame refused to display
blame content even if the path provided does match an existing
blob on said revision.
$ git status
On branch hide
Changes not staged for commit:
(use "git add/rm ..." to update what will be committed)
(use "git checkout -- ..." to discard changes in working directory)
deleted:testfile1.txt
no changes added to commit (use "git add" and/or "git commit -a")
Before:
$ git blame testfile1.txt
fatal: cannot stat path 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
fatal: cannot stat path 'testfile1.txt': No such file or directory
After:
$ git blame testfile1.txt
fatal: Cannot lstat 'testfile1.txt': No such file or directory
$ git blame testfile1.txt HEAD
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 1)
testfile 2
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 2)
^da1a96f testfile2.txt (Edmundo Carmona Antoranz 2015-11-10 17:52:00 -0600 3)
Some content
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/blame.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/builtin/blame.c b/builtin/blame.c
index 83612f5..db430d5 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -2683,8 +2683,6 @@ parse_done:
argv[argc - 1] = "--";
setup_work_tree();
- if (!file_exists(path))
- die_errno("cannot stat path '%s'", path);
}
revs.disable_stdin = 1;
--
2.6.2
--
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: What's cooking in git.git (Nov 2015, #02; Fri, 6)
On Fri, Nov 6, 2015 at 5:41 PM, Junio C Hamano wrote: > I'll be offline for a few weeks, and Jeff King graciously agreed to > help shepherd the project forward in the meantime as an interim > maintainer. Please be gentle. Be gentle? To Jeff??? But he's an ass Just kidding, man! Way to go, Peff! Not gonna offer my kind help cause there's not much help I can offer at the time given my little knowledge of the... 'business'. And Junio, if it's vacations, enjoy! Pura vida, mae! (That'd be the tico version of 'Hakuna Matata!' 'tico' being 'Costa Rican' 'Costa Rica' being) Cheers! -- 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: About global --progress option
On Thu, Nov 5, 2015 at 12:11 AM, Junio C Hamano wrote: > Besides, I am not convinced that you are bringing in a net positive > value by allowing "git --no-progress clone". You would need to > think what to do when you get two conflicting options (e.g. should > "git --no-progress clone --progress" give progress? Why?), you > would need to explain to the users why the resulting code works the > way you made it work (most likely, "do nothing special") when the > global one is given to a command that does not give any progress > output, and you would need to make sure whatever answers you would > give to these questions are implemented consistently. And you would > need more code to do so. It is unclear to me what value the end > users get out of all that effort, if any, and if the value offered > to the end users outweigh the added complexity, additional code that > has to be maintained, and additional risk to introduce unnecessary > bugs. The contradictory case of git --progress whatever --no-progress (or viceversa) was going to be my follow-up question. Dude, don't get too far ahead in the conversation :-) On the technical side, I think the global --progress option (and removing the option from the builtins) would not add complexity but the opposite because setting the flag would be done at the "main git" level and then all the builtins would just forget about it and would use progress regardless (cause deciding _if_ progress should be shown or not won't be up to them anymore) so the code from the builtins to support the option would be gone. It would certainly be more complex while keeping global and builtin options alive. Anyway, I do understand your concerns and will stand down on the topic (as in global --progress who???). > A lot more useful thing to do in the same area with a lot smaller > amount of effort would be to find an operation that sometimes take a > long time to perform that does not show the progress report and > teach the codepath involved in the operation to show progress, I > would think. Sounds interesting and in-topic (different direction). You got a list of those paths that I could work on? At least a couple you can think off the top of your head? Cause I can't complain about the progress I get on the workflows I follow (although I could keep a closer look to try to catch some operation I know is taking place and is not showing any progress and I'm used to it and so I don't complain at the lack of progress). Right now I'm thinking about cherry-picking... sometimes it can take a few seconds (or more seconds you have to see the performance of some of the boxes that I work on) and getting some progress there would be nice. Will take a look at it. Nice way to get familiarized with the code, by the way. Thank you very much, Junio! -- 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
About global --progress option
Hi, everybody! Coming back from my patch about the --[no-]progress option for checkout (it's already in next, J), I'd like to build into the idea of the single --[no-]progress option for git as a whole. So, in order to know what/how things should be developed, let's start with a tiny survey Which would be the correct development path? - Two-step process: First step, implement global --[no-]progress at the git level and also support the option from the builtins that laready have it. Let it live like that for some releases (so that people have the time to dump using the builtin option and start using the global option) and then on the second step dump the builtin options and keep the global one. or - A single step that would remove --[no-]progress from all builtins that support it and would place it as a global git option? Thanks in advance. -- 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
[PATCH v8] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 14 --
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..5e5273e 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless `--quiet`
+ is specified. This flag enables progress reporting even if not
+ attached to a terminal, regardless of `--quiet`.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..e346f52 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,13 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ if (opts.show_progress < 0) {
+ if (opts.quiet)
+ opts.show_progress = 0;
+ else
+ opts.show_progress = isatty(2);
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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: [PATCH v7] checkout: add --progress option
On Sun, Nov 1, 2015 at 3:15 PM, Eric Sunshine wrote: >> +--[no-]progress:: >> + Progress status is reported on the standard error stream >> + by default when it is attached to a terminal, unless `--quiet` >> + is specified. This flag enables progress reporting even if not >> + attached to a terminal, regardless of `--quite`. > > s/quite/quiet/ hahaha. So close!!! v8 on the pipeline. -- 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: [PATCH v6] checkout: add --progress option
On Sun, Nov 1, 2015 at 3:06 PM, Eric Sunshine wrote: >> +--[no-]progress:: >> + Progress status is reported on the standard error stream >> + by default when it is attached to a terminal, unless --quiet >> + is specified. This flag enables progress reporting even if not >> + attached to a terminal, regardless of -q. > > The mix of -q and --quiet is inconsistent and potentially confusing. I > suspect that your intention was to hint that they are equivalent, > however, the reader who is not familiar with -q as an alias of --quiet > may now be forced to look up both options, rather than just one, only > to discover that they are the same, thus potentially requiring extra > effort. It probably would be better to consistently use --quiet. > > Also, quoting with backticks is recommended: `--quite` > > The rest of the patch looks good. Good! Going for v7. -- 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
[PATCH v7] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 14 --
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..9973cf6 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless `--quiet`
+ is specified. This flag enables progress reporting even if not
+ attached to a terminal, regardless of `--quite`.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..e346f52 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,13 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ if (opts.show_progress < 0) {
+ if (opts.quiet)
+ opts.show_progress = 0;
+ else
+ opts.show_progress = isatty(2);
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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
[PATCH v6] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 14 --
2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..c24c8ee 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--[no-]progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless --quiet
+ is specified. This flag enables progress reporting even if not
+ attached to a terminal, regardless of -q.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..e346f52 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,13 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ if (opts.show_progress < 0) {
+ if (opts.quiet)
+ opts.show_progress = 0;
+ else
+ opts.show_progress = isatty(2);
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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: [PATCH v4] checkout: add --progress option
On Sun, Nov 1, 2015 at 2:15 PM, Eric Sunshine wrote: > > Progress status is reported on the standard error stream by > default when it is attached to a terminal, unless `--quiet` is > specified. This flag enables progress reporting even if not > attached to a terminal, regardless of `--quiet`. Sounds good to me. Sending another full patch. -- 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: [PATCH v4] checkout: add --progress option
On Sun, Nov 1, 2015 at 1:29 PM, Eric Sunshine wrote: >>> > +--progress:: >>> > + Progress status is reported on the standard error stream >>> > + by default when it is attached to a terminal, unless -q >>> > + is specified. This flag forces progress status even if the >>> > + standard error stream is not directed to a terminal. Before I send a new full patch, could you guys tell me if you find this ok? -q:: --quiet:: Quiet, suppress feedback messages. Progress can skip this option. Read the information about --progress --[no-]progress:: Progress status is reported on the standard error stream by default when it is attached to a terminal, unless --quiet or -q is specified. This flag forces progress status even if the standard error stream is not directed to a terminal and overrides the --quiet or -q options. Thank you all, guys! -- 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: [PATCH v5] checkout: add --progress option
Oh, man... I'm sorry... I didn't notice them. Let me go over them to see what we can do. On Sun, Nov 1, 2015 at 1:13 PM, Eric Sunshine wrote: > I'll assume that you simply overlooked the remainder of my v4 review > comments, so I'll merely provide a reference to them[1] rather than > repeating myself. If that assumption is incorrect, please do have the > courtesy to state that you disagree with review comments and to > explain your position, otherwise reviewers will feel that their > efforts are wasted. -- 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
[PATCH v5] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 25 +++--
2 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..93ba35a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless -q
+ is specified. This flag forces progress status even if the
+ standard error stream is not directed to a terminal.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..65b8b90 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,24 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ /*
+* Final processing of show_progress
+* - User selected --progress: show progress
+* - user selected --no-progress: skip progress
+* - User didn't specify:
+* (check rules in order till finding the first matching one)
+* - user selected --quiet: skip progress
+* - stderr is connected to a terminal: show progress
+* - fallback: skip progress
+*/
+ if (opts.show_progress < 0) {
+ /* user didn't specify --[no-]progress */
+ if (opts.quiet)
+ opts.show_progress = 0;
+ else
+ opts.show_progress = isatty(2);
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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: [PATCH v4] checkout: add --progress option
On Sun, Nov 1, 2015 at 11:52 AM, Eric Sunshine wrote:
>> + if (opts.quiet) {
>> + opts.show_progress = 0;
>> + } else {
>> + opts.show_progress = isatty(2);
>> + }
>
> Style: drop unnecessary braces
>
Ok. WIll do!
--
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
[PATCH v4] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
Signed-off-by: Edmundo Carmona Antoranz
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 26 --
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..93ba35a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless -q
+ is specified. This flag forces progress status even if the
+ standard error stream is not directed to a terminal.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..c3b8e5d 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,25 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ /*
+* Final processing of show_progress
+* - User selected --progress: show progress
+* - user selected --no-progress: skip progress
+* - User didn't specify:
+* (check rules in order till finding the first matching one)
+* - user selected --quiet: skip progress
+* - stderr is connected to a terminal: show progress
+* - fallback: skip progress
+*/
+ if (opts.show_progress < 0) {
+ /* user didn't specify --[no-]progress */
+ if (opts.quiet) {
+ opts.show_progress = 0;
+ } else {
+ opts.show_progress = isatty(2);
+ }
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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: [PATCH v3] checkout: add --progress option
On Sun, Nov 1, 2015 at 11:22 AM, Edmundo Carmona Antoranz
wrote:
> + /*
> +* Final processing of show_progress
> +* - User selected --progress: show progress
> +* - user selected --no-progress: skip progress
> +* - User didn't specify:
> +* (check rules in order till finding the first matching one)
> +* - user selected --quiet: skip progress
> +* - stderr is connected to a terminal: show progress
> +* - fallback: skip progress
> +*/
> + if (opts.show_progress < 0) {
> + /* user selected --progress or didn't specify */
> + if (opts.quiet) {
> + opts.show_progress = 0;
> + } else {
> + opts.show_progress = isatty(2);
> + }
> + }
> +
/* user selected --progress or didn't specify */
This comment is not correct. Disregard this patch.
--
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
[PATCH v3] checkout: add --progress option
Under normal circumstances, and like other git commands,
git checkout will write progress info to stderr if
attached to a terminal. This option allows progress
to be forced even if not using a terminal. Also,
progress can be skipped if using option --no-progress.
---
Documentation/git-checkout.txt | 6 ++
builtin/checkout.c | 26 --
2 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt
index e269fb1..93ba35a 100644
--- a/Documentation/git-checkout.txt
+++ b/Documentation/git-checkout.txt
@@ -107,6 +107,12 @@ OPTIONS
--quiet::
Quiet, suppress feedback messages.
+--progress::
+ Progress status is reported on the standard error stream
+ by default when it is attached to a terminal, unless -q
+ is specified. This flag forces progress status even if the
+ standard error stream is not directed to a terminal.
+
-f::
--force::
When switching branches, proceed even if the index or the
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bc703c0..1bc4efe 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -37,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -417,7 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- opts.verbose_update = !o->quiet && isatty(2);
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -501,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1156,6 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1163,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1172,6 +1175,25 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ /*
+* Final processing of show_progress
+* - User selected --progress: show progress
+* - user selected --no-progress: skip progress
+* - User didn't specify:
+* (check rules in order till finding the first matching one)
+* - user selected --quiet: skip progress
+* - stderr is connected to a terminal: show progress
+* - fallback: skip progress
+*/
+ if (opts.show_progress < 0) {
+ /* user selected --progress or didn't specify */
+ if (opts.quiet) {
+ opts.show_progress = 0;
+ } else {
+ opts.show_progress = isatty(2);
+ }
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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: [PATCH] checkout: add --progress option
On Sat, Oct 31, 2015 at 6:45 PM, Eric Sunshine wrote: > Patches in 'next' are pretty much set in stone, and those in 'master' > definitely are, but 'pu' is volatile, so just send the entire patch > revised, tagged v2 (or v3, etc.), rather than sending a patch to fix > what is in 'pu', and Junio will replace the previous version with the > new one. > Will do! -- 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
[PATCH v2] checkout: refactor of --progress option
- removed static variable show_progress
- processing if progress will be shown or not right
after running parse_options()
Signed-off-by: Edmundo Carmona Antoranz
---
builtin/checkout.c | 38 +-
1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e28c36b..81c9e6f 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -27,8 +27,6 @@ static const char * const checkout_usage[] = {
NULL,
};
-static int option_progress = -1;
-
struct checkout_opts {
int patch_mode;
int quiet;
@@ -39,6 +37,7 @@ struct checkout_opts {
int overwrite_ignore;
int ignore_skipworktree;
int ignore_other_worktrees;
+ int show_progress;
const char *new_branch;
const char *new_branch_force;
@@ -419,19 +418,7 @@ static int reset_tree(struct tree *tree, const struct
checkout_opts *o,
opts.reset = 1;
opts.merge = 1;
opts.fn = oneway_merge;
- /**
-* Rules to display progress:
-* -q is selected
-* no verbiage
-* -q is _not_ selected and --no-progress _is_ selected,
-* progress will be skipped
-* -q is _not_ selected and --progress _is_ selected,
-* progress will be printed to stderr
-* -q is _not_ selected and --progress is 'undefined'
-* progress will be printed to stderr _if_ working on a terminal
-*/
- opts.verbose_update = !o->quiet && (option_progress > 0 ||
- (option_progress < 0 && isatty(2)));
+ opts.verbose_update = o->show_progress;
opts.src_index = &the_index;
opts.dst_index = &the_index;
parse_tree(tree);
@@ -515,7 +502,7 @@ static int merge_working_tree(const struct checkout_opts
*opts,
topts.update = 1;
topts.merge = 1;
topts.gently = opts->merge && old->commit;
- topts.verbose_update = !opts->quiet && isatty(2);
+ topts.verbose_update = opts->show_progress;
topts.fn = twoway_merge;
if (opts->overwrite_ignore) {
topts.dir = xcalloc(1, sizeof(*topts.dir));
@@ -1170,7 +1157,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
N_("second guess 'git checkout
'")),
OPT_BOOL(0, "ignore-other-worktrees",
&opts.ignore_other_worktrees,
N_("do not check if another worktree is holding the
given ref")),
- OPT_BOOL(0, "progress", &option_progress, N_("force progress
reporting")),
+ OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress
reporting")),
OPT_END(),
};
@@ -1178,6 +1165,7 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
memset(&new, 0, sizeof(new));
opts.overwrite_ignore = 1;
opts.prefix = prefix;
+ opts.show_progress = -1;
gitmodules_config();
git_config(git_checkout_config, &opts);
@@ -1187,6 +1175,22 @@ int cmd_checkout(int argc, const char **argv, const char
*prefix)
argc = parse_options(argc, argv, prefix, options, checkout_usage,
PARSE_OPT_KEEP_DASHDASH);
+ /*
+* Final processing of show_progress
+* Any of these 3 conditions will make progress output be skipped:
+* - selected --quiet
+* - selected --no-progress
+* - didn't select --progress and not working on a terminal
+*/
+ if (opts.show_progress) {
+ /* user selected --progress or didn't specify */
+ if (opts.quiet) {
+ opts.show_progress = 0;
+ } else if (opts.show_progress < 0) {
+ opts.show_progress = isatty(2);
+ }
+ }
+
if (conflict_style) {
opts.merge = 1; /* implied */
git_xmerge_config("merge.conflictstyle", conflict_style, NULL);
--
2.6.1
--
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
