* lk/more-helpful-status-hints (was Re: What's cooking in git.git (Jun 2012, #03; Wed, 13))
2012-06-13 23:01 What's cooking in git.git (Jun 2012, #03; Wed, 13) Junio C Hamano
@ 2012-06-14 8:18 ` Matthieu Moy
2012-06-14 8:20 ` [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically y
2012-06-14 8:20 ` y
2 siblings, 0 replies; 8+ messages in thread
From: Matthieu Moy @ 2012-06-14 8:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, Valentin DUPERRAY, Franck JONAS, Lucien KONG, Thomas NGUY,
Huynh Khoi Nguyen NGUYEN
Junio C Hamano <gitster@pobox.com> writes:
> * lk/more-helpful-status-hints (2012-06-11) 5 commits
> - fixup! status: better advices when splitting a commit (during rebase -i)
> - status: better advices when splitting a commit (during rebase -i)
> - status: don't suggest "git rm" or "git add" if not appropriate
> - t7512-status-help.sh: better advices for git status
> - wt-status.*: better advices for git status added
>
> Almost there, modulo minor details pointed out by Matthieu.
The details I pointed out earlier are already fixed. I went through the
serie again, and suggest two more minor fixups:
fixup! reword message when splitting a commit.
fixup! don't allocate struct wt_status_state dynamically
patches follow.
> * lk/rebase-i-x (2012-06-13) 1 commit
> - rebase -i: teach "--exec <cmd>"
>
> Adds -x <cmd> to "rebase -i" to insert "exec <cmd>" after each
> commit in the resulting history.
>
> Will merge to next after 1.7.11.
Cool!
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically
2012-06-13 23:01 What's cooking in git.git (Jun 2012, #03; Wed, 13) Junio C Hamano
2012-06-14 8:18 ` lk/more-helpful-status-hints (was Re: What's cooking in git.git (Jun 2012, #03; Wed, 13)) Matthieu Moy
@ 2012-06-14 8:20 ` y
2012-06-14 17:13 ` Junio C Hamano
2012-06-14 8:20 ` y
2 siblings, 1 reply; 8+ messages in thread
From: y @ 2012-06-14 8:20 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
From: Matthieu Moy <Matthieu.Moy@imag.fr>
The common
void function() {
struct wt_status_state *state = calloc(...);
...
free(state);
}
is essentially a less efficient, and more error prone way of allocating a
variable on the stack (plus, the calloc should have been a xcalloc).
Replace it with an on-stack variable.
While we're there, also replace the individual initializations of fields
with memset(..., 0, ...).
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
(BTW, I didn't find a way to have both --autosquash-compliant and
meaningfull titles)
wt-status.c | 49 +++++++++++++++++++++----------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index ed28b4f..e65716d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -932,49 +932,42 @@ static void show_bisect_in_progress(struct wt_status *s,
static void wt_status_print_state(struct wt_status *s)
{
const char *state_color = color(WT_STATUS_IN_PROGRESS, s);
- struct wt_status_state *state = calloc(1, sizeof(*state));
+ struct wt_status_state state;
struct stat st;
- state->merge_in_progress = 0;
- state->am_in_progress = 0;
- state->am_empty_patch = 0;
- state->rebase_in_progress = 0;
- state->rebase_interactive_in_progress = 0;
- state->cherry_pick_in_progress = 0;
- state->bisect_in_progress = 0;
+ memset(&state, 0, sizeof(state));
if (!stat(git_path("MERGE_HEAD"), &st)) {
- state->merge_in_progress = 1;
+ state.merge_in_progress = 1;
} else if (!stat(git_path("rebase-apply"), &st)) {
if (!stat(git_path("rebase-apply/applying"), &st)) {
- state->am_in_progress = 1;
+ state.am_in_progress = 1;
if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
- state->am_empty_patch = 1;
+ state.am_empty_patch = 1;
} else {
- state->rebase_in_progress = 1;
+ state.rebase_in_progress = 1;
}
} else if (!stat(git_path("rebase-merge"), &st)) {
if (!stat(git_path("rebase-merge/interactive"), &st))
- state->rebase_interactive_in_progress = 1;
+ state.rebase_interactive_in_progress = 1;
else
- state->rebase_in_progress = 1;
+ state.rebase_in_progress = 1;
} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
- state->cherry_pick_in_progress = 1;
+ state.cherry_pick_in_progress = 1;
}
if (!stat(git_path("BISECT_LOG"), &st))
- state->bisect_in_progress = 1;
-
- if (state->merge_in_progress)
- show_merge_in_progress(s, state, state_color);
- else if (state->am_in_progress)
- show_am_in_progress(s, state, state_color);
- else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
- show_rebase_in_progress(s, state, state_color);
- else if (state->cherry_pick_in_progress)
- show_cherry_pick_in_progress(s, state, state_color);
- if (state->bisect_in_progress)
- show_bisect_in_progress(s, state, state_color);
- free(state);
+ state.bisect_in_progress = 1;
+
+ if (state.merge_in_progress)
+ show_merge_in_progress(s, &state, state_color);
+ else if (state.am_in_progress)
+ show_am_in_progress(s, &state, state_color);
+ else if (state.rebase_in_progress || state.rebase_interactive_in_progress)
+ show_rebase_in_progress(s, &state, state_color);
+ else if (state.cherry_pick_in_progress)
+ show_cherry_pick_in_progress(s, &state, state_color);
+ if (state.bisect_in_progress)
+ show_bisect_in_progress(s, &state, state_color);
}
void wt_status_print(struct wt_status *s)
--
1.7.11.rc0.57.g84a04c7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically
2012-06-14 8:20 ` [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically y
@ 2012-06-14 17:13 ` Junio C Hamano
0 siblings, 0 replies; 8+ messages in thread
From: Junio C Hamano @ 2012-06-14 17:13 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git
y@imag.fr writes:
> From: Matthieu Moy <Matthieu.Moy@imag.fr>
>
> The common
>
> void function() {
> struct wt_status_state *state = calloc(...);
> ...
> free(state);
> }
>
> is essentially a less efficient, and more error prone way of allocating a
> variable on the stack (plus, the calloc should have been a xcalloc).
> Replace it with an on-stack variable.
>
> While we're there, also replace the individual initializations of fields
> with memset(..., 0, ...).
>
> Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
> ---
>
> (BTW, I didn't find a way to have both --autosquash-compliant and
> meaningfull titles)
>
> wt-status.c | 49 +++++++++++++++++++++----------------------------
> 1 file changed, 21 insertions(+), 28 deletions(-)
Thanks; much better.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically
2012-06-13 23:01 What's cooking in git.git (Jun 2012, #03; Wed, 13) Junio C Hamano
2012-06-14 8:18 ` lk/more-helpful-status-hints (was Re: What's cooking in git.git (Jun 2012, #03; Wed, 13)) Matthieu Moy
2012-06-14 8:20 ` [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically y
@ 2012-06-14 8:20 ` y
2012-06-14 8:20 ` [PATCH 2/2] fixup! 38388c8 reword message when splitting a commit y
` (2 more replies)
2 siblings, 3 replies; 8+ messages in thread
From: y @ 2012-06-14 8:20 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
From: Matthieu Moy <Matthieu.Moy@imag.fr>
The common
void function() {
struct wt_status_state *state = calloc(...);
...
free(state);
}
is essentially a less efficient, and more error prone way of allocating a
variable on the stack (plus, the calloc should have been a xcalloc).
Replace it with an on-stack variable.
While we're there, also replace the individual initializations of fields
with memset(..., 0, ...).
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
(BTW, I didn't find a way to have both --autosquash-compliant and
meaningfull titles)
wt-status.c | 49 +++++++++++++++++++++----------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/wt-status.c b/wt-status.c
index ed28b4f..e65716d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -932,49 +932,42 @@ static void show_bisect_in_progress(struct wt_status *s,
static void wt_status_print_state(struct wt_status *s)
{
const char *state_color = color(WT_STATUS_IN_PROGRESS, s);
- struct wt_status_state *state = calloc(1, sizeof(*state));
+ struct wt_status_state state;
struct stat st;
- state->merge_in_progress = 0;
- state->am_in_progress = 0;
- state->am_empty_patch = 0;
- state->rebase_in_progress = 0;
- state->rebase_interactive_in_progress = 0;
- state->cherry_pick_in_progress = 0;
- state->bisect_in_progress = 0;
+ memset(&state, 0, sizeof(state));
if (!stat(git_path("MERGE_HEAD"), &st)) {
- state->merge_in_progress = 1;
+ state.merge_in_progress = 1;
} else if (!stat(git_path("rebase-apply"), &st)) {
if (!stat(git_path("rebase-apply/applying"), &st)) {
- state->am_in_progress = 1;
+ state.am_in_progress = 1;
if (!stat(git_path("rebase-apply/patch"), &st) && !st.st_size)
- state->am_empty_patch = 1;
+ state.am_empty_patch = 1;
} else {
- state->rebase_in_progress = 1;
+ state.rebase_in_progress = 1;
}
} else if (!stat(git_path("rebase-merge"), &st)) {
if (!stat(git_path("rebase-merge/interactive"), &st))
- state->rebase_interactive_in_progress = 1;
+ state.rebase_interactive_in_progress = 1;
else
- state->rebase_in_progress = 1;
+ state.rebase_in_progress = 1;
} else if (!stat(git_path("CHERRY_PICK_HEAD"), &st)) {
- state->cherry_pick_in_progress = 1;
+ state.cherry_pick_in_progress = 1;
}
if (!stat(git_path("BISECT_LOG"), &st))
- state->bisect_in_progress = 1;
-
- if (state->merge_in_progress)
- show_merge_in_progress(s, state, state_color);
- else if (state->am_in_progress)
- show_am_in_progress(s, state, state_color);
- else if (state->rebase_in_progress || state->rebase_interactive_in_progress)
- show_rebase_in_progress(s, state, state_color);
- else if (state->cherry_pick_in_progress)
- show_cherry_pick_in_progress(s, state, state_color);
- if (state->bisect_in_progress)
- show_bisect_in_progress(s, state, state_color);
- free(state);
+ state.bisect_in_progress = 1;
+
+ if (state.merge_in_progress)
+ show_merge_in_progress(s, &state, state_color);
+ else if (state.am_in_progress)
+ show_am_in_progress(s, &state, state_color);
+ else if (state.rebase_in_progress || state.rebase_interactive_in_progress)
+ show_rebase_in_progress(s, &state, state_color);
+ else if (state.cherry_pick_in_progress)
+ show_cherry_pick_in_progress(s, &state, state_color);
+ if (state.bisect_in_progress)
+ show_bisect_in_progress(s, &state, state_color);
}
void wt_status_print(struct wt_status *s)
--
1.7.11.rc0.57.g84a04c7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] fixup! 38388c8 reword message when splitting a commit.
2012-06-14 8:20 ` y
@ 2012-06-14 8:20 ` y
2012-06-14 8:20 ` y
2012-06-14 8:23 ` [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically Matthieu Moy
2 siblings, 0 replies; 8+ messages in thread
From: y @ 2012-06-14 8:20 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
From: Matthieu Moy <Matthieu.Moy@imag.fr>
The "splitting a commit" is a sub-case of the "rebase" case, hence,
mention rebase like this:
# You are currently splitting a commit during a rebase.
This makes the message consistant with "editing a commit":
# You are currently editing a commit during a rebase.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
t/t7512-status-help.sh | 8 ++++----
wt-status.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 3ce384a..b3f6eb9 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -210,7 +210,7 @@ test_expect_success 'status when splitting a commit' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -290,7 +290,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -359,7 +359,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -433,7 +433,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
diff --git a/wt-status.c b/wt-status.c
index e65716d..c749267 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -886,7 +886,7 @@ static void show_rebase_in_progress(struct wt_status *s,
status_printf_ln(s, color,
_(" (all conflicts fixed: run \"git rebase --continue\")"));
} else if (split_commit_in_progress(s)) {
- status_printf_ln(s, color, _("You are currently splitting a commit."));
+ status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
if (advice_status_hints)
status_printf_ln(s, color,
_(" (Once your working directory is clean, run \"git rebase --continue\")"));
--
1.7.11.rc0.57.g84a04c7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] fixup! 38388c8 reword message when splitting a commit.
2012-06-14 8:20 ` y
2012-06-14 8:20 ` [PATCH 2/2] fixup! 38388c8 reword message when splitting a commit y
@ 2012-06-14 8:20 ` y
2012-06-14 8:23 ` [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically Matthieu Moy
2 siblings, 0 replies; 8+ messages in thread
From: y @ 2012-06-14 8:20 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
From: Matthieu Moy <Matthieu.Moy@imag.fr>
The "splitting a commit" is a sub-case of the "rebase" case, hence,
mention rebase like this:
# You are currently splitting a commit during a rebase.
This makes the message consistant with "editing a commit":
# You are currently editing a commit during a rebase.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
t/t7512-status-help.sh | 8 ++++----
wt-status.c | 2 +-
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t7512-status-help.sh b/t/t7512-status-help.sh
index 3ce384a..b3f6eb9 100755
--- a/t/t7512-status-help.sh
+++ b/t/t7512-status-help.sh
@@ -210,7 +210,7 @@ test_expect_success 'status when splitting a commit' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -290,7 +290,7 @@ test_expect_success 'status: (continue first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -359,7 +359,7 @@ test_expect_success 'status: (amend first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
@@ -433,7 +433,7 @@ test_expect_success 'status: (split first edit) second edit and split' '
git reset HEAD^ &&
cat >expected <<-\EOF &&
# Not currently on any branch.
- # You are currently splitting a commit.
+ # You are currently splitting a commit during a rebase.
# (Once your working directory is clean, run "git rebase --continue")
#
# Changes not staged for commit:
diff --git a/wt-status.c b/wt-status.c
index e65716d..c749267 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -886,7 +886,7 @@ static void show_rebase_in_progress(struct wt_status *s,
status_printf_ln(s, color,
_(" (all conflicts fixed: run \"git rebase --continue\")"));
} else if (split_commit_in_progress(s)) {
- status_printf_ln(s, color, _("You are currently splitting a commit."));
+ status_printf_ln(s, color, _("You are currently splitting a commit during a rebase."));
if (advice_status_hints)
status_printf_ln(s, color,
_(" (Once your working directory is clean, run \"git rebase --continue\")"));
--
1.7.11.rc0.57.g84a04c7
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] fixup! 8e00b48 don't allocate struct wt_status_state dynamically
2012-06-14 8:20 ` y
2012-06-14 8:20 ` [PATCH 2/2] fixup! 38388c8 reword message when splitting a commit y
2012-06-14 8:20 ` y
@ 2012-06-14 8:23 ` Matthieu Moy
2 siblings, 0 replies; 8+ messages in thread
From: Matthieu Moy @ 2012-06-14 8:23 UTC (permalink / raw)
To: git, gitster
> To: y@imag.fr
Oops, sorry, I tried answering "yes" to the wrong question. Obviously,
this is not my real email address :-(.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 8+ messages in thread