* Re: [PATCH] Make sure show_all_errors when using porcelain error messages
2010-09-01 17:04 ` Matthieu Moy
@ 2010-09-01 17:54 ` Junio C Hamano
2010-09-02 11:57 ` [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed Matthieu Moy
` (3 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2010-09-01 17:54 UTC (permalink / raw)
To: Matthieu Moy; +Cc: Junio C Hamano, git
Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>>
>>> opts.show_all_errors controls the number of errors that can be displayed,
>>> and set_porcelain_error_msgs gives the format. But the formatting would
>>> be incorrect if set_porcelain_error_msgs is called without setting
>>> opts.show_all_errors.
>>
>> That makes it sound like a design bug of set_porcelain_error_msgs(), in
>> that the caller _cannot_ choose to stop at the first error if it wants to
>> use friendlier message than the plumbing one.
>
> You're right that the caller cannot stop at the first error and get
> friendly message, but I don't think this is a bug.
I am not saying it is a bug. The description makes it sound like it is a
bug and I was wondering if it can be worded better not to do so.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed
2010-09-01 17:04 ` Matthieu Moy
2010-09-01 17:54 ` Junio C Hamano
@ 2010-09-02 11:57 ` Matthieu Moy
2010-09-03 14:18 ` Jim Meyering
2010-09-02 11:57 ` [PATCH 1/3] Move set_porcelain_error_msgs to unpack-trees.c and rename it Matthieu Moy
` (2 subsequent siblings)
4 siblings, 1 reply; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 11:57 UTC (permalink / raw)
To: git, gitster; +Cc: Jim Meyering, Matthieu Moy
So, after my toy untested patch, here's a cleaner version. I splitted
it into really small patches, to avoid modifying code while I move it,
but Junio, feel free to squash 1/3 and 2/3 if you prefer.
This fixes a regression, so this should be in the next release if the
patch is OK.
Jim: can you confirm if it fixes your problem?
Matthieu Moy (3):
Move set_porcelain_error_msgs to unpack-trees.c and rename it
setup_unpack_trees_porcelain: take the whole options struct as
parameter
Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
builtin/checkout.c | 3 +-
builtin/merge.c | 3 +-
merge-recursive.c | 46 +-----------------------------------------
merge-recursive.h | 6 -----
unpack-trees.c | 56 +++++++++++++++++++++++++++++++++++++++++++++------
unpack-trees.h | 7 ++++++
6 files changed, 59 insertions(+), 62 deletions(-)
--
1.7.2.2.175.ga619d.dirty
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed
2010-09-02 11:57 ` [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed Matthieu Moy
@ 2010-09-03 14:18 ` Jim Meyering
2010-09-03 15:25 ` [PATCH] t7609-merge-co-error-msgs: test non-fast forward case too Matthieu Moy
0 siblings, 1 reply; 18+ messages in thread
From: Jim Meyering @ 2010-09-03 14:18 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, gitster
Matthieu Moy wrote:
> So, after my toy untested patch, here's a cleaner version. I splitted
> it into really small patches, to avoid modifying code while I move it,
> but Junio, feel free to squash 1/3 and 2/3 if you prefer.
>
> This fixes a regression, so this should be in the next release if the
> patch is OK.
>
> Jim: can you confirm if it fixes your problem?
Hi Matthieu,
Thanks for all of those patches.
Unfortunately, I was unable to reproduce the state
that provoked my problem.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] t7609-merge-co-error-msgs: test non-fast forward case too.
2010-09-03 14:18 ` Jim Meyering
@ 2010-09-03 15:25 ` Matthieu Moy
0 siblings, 0 replies; 18+ messages in thread
From: Matthieu Moy @ 2010-09-03 15:25 UTC (permalink / raw)
To: git, gitster; +Cc: Jim Meyering, Matthieu Moy
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
This comes on top of the patch serie, and adds a test to make sure the
bug doesn't come back.
I did check manually that the git of git.git doesn't pass this (while
the one after the patch serie does).
t/t7609-merge-co-error-msgs.sh | 12 ++++++++++--
1 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/t/t7609-merge-co-error-msgs.sh b/t/t7609-merge-co-error-msgs.sh
index 1a109b4..6b58299 100755
--- a/t/t7609-merge-co-error-msgs.sh
+++ b/t/t7609-merge-co-error-msgs.sh
@@ -34,9 +34,17 @@ error: The following untracked working tree files would be overwritten by merge:
Please move or remove them before you can merge.
EOF
-test_expect_success 'untracked files overwritten by merge' '
+test_expect_success 'untracked files overwritten by merge (fast and non-fast forward)' '
test_must_fail git merge branch 2>out &&
- test_cmp out expect
+ test_cmp out expect &&
+ git commit --allow-empty -m empty &&
+ (
+ GIT_MERGE_VERBOSITY=0 &&
+ export GIT_MERGE_VERBOSITY &&
+ test_must_fail git merge branch 2>out2
+ ) &&
+ test_cmp out2 expect &&
+ git reset --hard HEAD^
'
cat >expect <<\EOF
--
1.7.2.2.284.g95cc.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/3] Move set_porcelain_error_msgs to unpack-trees.c and rename it
2010-09-01 17:04 ` Matthieu Moy
2010-09-01 17:54 ` Junio C Hamano
2010-09-02 11:57 ` [PATCH 0/3] (hopefully) Proper fix to set show_all_errors where needed Matthieu Moy
@ 2010-09-02 11:57 ` Matthieu Moy
2010-09-02 11:57 ` [PATCH 2/3] setup_unpack_trees_porcelain: take the whole options struct as parameter Matthieu Moy
2010-09-02 11:57 ` [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain() Matthieu Moy
4 siblings, 0 replies; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 11:57 UTC (permalink / raw)
To: git, gitster; +Cc: Jim Meyering, Matthieu Moy
The function is currently dealing only with error messages, but the
intent of calling it is really to notify the unpack-tree mechanics that
it is running in porcelain mode.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
builtin/checkout.c | 2 +-
builtin/merge.c | 2 +-
merge-recursive.c | 46 +---------------------------------------------
merge-recursive.h | 6 ------
unpack-trees.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
unpack-trees.h | 6 ++++++
6 files changed, 54 insertions(+), 54 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 7250e5c..e5c0ef0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -376,7 +376,7 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.src_index = &the_index;
topts.dst_index = &the_index;
- set_porcelain_error_msgs(topts.msgs, "checkout");
+ setup_unpack_trees_porcelain(topts.msgs, "checkout");
refresh_cache(REFRESH_QUIET);
diff --git a/builtin/merge.c b/builtin/merge.c
index 47e705b..da52b10 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -706,7 +706,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
opts.merge = 1;
opts.fn = twoway_merge;
opts.show_all_errors = 1;
- set_porcelain_error_msgs(opts.msgs, "merge");
+ setup_unpack_trees_porcelain(opts.msgs, "merge");
trees[nr_trees] = parse_tree_indirect(head);
if (!trees[nr_trees++])
diff --git a/merge-recursive.c b/merge-recursive.c
index df90be4..61e237b 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -180,7 +180,7 @@ static int git_merge_trees(int index_only,
opts.fn = threeway_merge;
opts.src_index = &the_index;
opts.dst_index = &the_index;
- set_porcelain_error_msgs(opts.msgs, "merge");
+ setup_unpack_trees_porcelain(opts.msgs, "merge");
init_tree_desc_from_tree(t+0, common);
init_tree_desc_from_tree(t+1, head);
@@ -1238,50 +1238,6 @@ static int process_df_entry(struct merge_options *o,
return clean_merge;
}
-void set_porcelain_error_msgs(const char **msgs, const char *cmd)
-{
- const char *msg;
- char *tmp;
- const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
- if (advice_commit_before_merge)
- msg = "Your local changes to the following files would be overwritten by %s:\n%%s"
- "Please, commit your changes or stash them before you can %s.";
- else
- msg = "Your local changes to the following files would be overwritten by %s:\n%%s";
- tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);
- sprintf(tmp, msg, cmd, cmd2);
- msgs[ERROR_WOULD_OVERWRITE] = tmp;
- msgs[ERROR_NOT_UPTODATE_FILE] = tmp;
-
- msgs[ERROR_NOT_UPTODATE_DIR] =
- "Updating the following directories would lose untracked files in it:\n%s";
-
- if (advice_commit_before_merge)
- msg = "The following untracked working tree files would be %s by %s:\n%%s"
- "Please move or remove them before you can %s.";
- else
- msg = "The following untracked working tree files would be %s by %s:\n%%s";
- tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4);
- sprintf(tmp, msg, "removed", cmd, cmd2);
- msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp;
- tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("overwritten") + strlen(cmd2) - 4);
- sprintf(tmp, msg, "overwritten", cmd, cmd2);
- msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp;
-
- /*
- * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we
- * cannot easily display it as a list.
- */
- msgs[ERROR_BIND_OVERLAP] = "Entry '%s' overlaps with '%s'. Cannot bind.";
-
- msgs[ERROR_SPARSE_NOT_UPTODATE_FILE] =
- "Cannot update sparse checkout: the following entries are not up-to-date:\n%s";
- msgs[ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN] =
- "The following Working tree files would be overwritten by sparse checkout update:\n%s";
- msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
- "The following Working tree files would be removed by sparse checkout update:\n%s";
-}
-
int merge_trees(struct merge_options *o,
struct tree *head,
struct tree *merge,
diff --git a/merge-recursive.h b/merge-recursive.h
index 08f9850..f79917c 100644
--- a/merge-recursive.h
+++ b/merge-recursive.h
@@ -23,12 +23,6 @@ struct merge_options {
struct string_list current_directory_set;
};
-/*
- * Sets the list of user-friendly error messages to be used by the
- * command "cmd" (either merge or checkout)
- */
-void set_porcelain_error_msgs(const char **msgs, const char *cmd);
-
/* merge_trees() but with recursive ancestor consolidation */
int merge_recursive(struct merge_options *o,
struct commit *h1,
diff --git a/unpack-trees.c b/unpack-trees.c
index 3c7a7c9..4520aa0 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -14,7 +14,7 @@
* read-tree. Non-scripted Porcelain is not required to use these messages
* and in fact are encouraged to reword them to better suit their particular
* situation better. See how "git checkout" and "git merge" replaces
- * them using set_porcelain_error_msgs(), for example.
+ * them using setup_unpack_trees_porcelain(), for example.
*/
const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
/* ERROR_WOULD_OVERWRITE */
@@ -50,6 +50,50 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
? ((o)->msgs[(type)]) \
: (unpack_plumbing_errors[(type)]) )
+void setup_unpack_trees_porcelain(const char **msgs, const char *cmd)
+{
+ const char *msg;
+ char *tmp;
+ const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
+ if (advice_commit_before_merge)
+ msg = "Your local changes to the following files would be overwritten by %s:\n%%s"
+ "Please, commit your changes or stash them before you can %s.";
+ else
+ msg = "Your local changes to the following files would be overwritten by %s:\n%%s";
+ tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen(cmd2) - 2);
+ sprintf(tmp, msg, cmd, cmd2);
+ msgs[ERROR_WOULD_OVERWRITE] = tmp;
+ msgs[ERROR_NOT_UPTODATE_FILE] = tmp;
+
+ msgs[ERROR_NOT_UPTODATE_DIR] =
+ "Updating the following directories would lose untracked files in it:\n%s";
+
+ if (advice_commit_before_merge)
+ msg = "The following untracked working tree files would be %s by %s:\n%%s"
+ "Please move or remove them before you can %s.";
+ else
+ msg = "The following untracked working tree files would be %s by %s:\n%%s";
+ tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("removed") + strlen(cmd2) - 4);
+ sprintf(tmp, msg, "removed", cmd, cmd2);
+ msgs[ERROR_WOULD_LOSE_UNTRACKED_REMOVED] = tmp;
+ tmp = xmalloc(strlen(msg) + strlen(cmd) + strlen("overwritten") + strlen(cmd2) - 4);
+ sprintf(tmp, msg, "overwritten", cmd, cmd2);
+ msgs[ERROR_WOULD_LOSE_UNTRACKED_OVERWRITTEN] = tmp;
+
+ /*
+ * Special case: ERROR_BIND_OVERLAP refers to a pair of paths, we
+ * cannot easily display it as a list.
+ */
+ msgs[ERROR_BIND_OVERLAP] = "Entry '%s' overlaps with '%s'. Cannot bind.";
+
+ msgs[ERROR_SPARSE_NOT_UPTODATE_FILE] =
+ "Cannot update sparse checkout: the following entries are not up-to-date:\n%s";
+ msgs[ERROR_WOULD_LOSE_ORPHANED_OVERWRITTEN] =
+ "The following Working tree files would be overwritten by sparse checkout update:\n%s";
+ msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
+ "The following Working tree files would be removed by sparse checkout update:\n%s";
+}
+
static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
unsigned int set, unsigned int clear)
{
diff --git a/unpack-trees.h b/unpack-trees.h
index 6e049b0..d62bba9 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -22,6 +22,12 @@ enum unpack_trees_error_types {
NB_UNPACK_TREES_ERROR_TYPES
};
+/*
+ * Sets the list of user-friendly error messages to be used by the
+ * command "cmd" (either merge or checkout)
+ */
+void setup_unpack_trees_porcelain(const char **msgs, const char *cmd);
+
struct rejected_paths_list {
char *path;
struct rejected_paths_list *next;
--
1.7.2.2.175.ga619d.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] setup_unpack_trees_porcelain: take the whole options struct as parameter
2010-09-01 17:04 ` Matthieu Moy
` (2 preceding siblings ...)
2010-09-02 11:57 ` [PATCH 1/3] Move set_porcelain_error_msgs to unpack-trees.c and rename it Matthieu Moy
@ 2010-09-02 11:57 ` Matthieu Moy
2010-09-02 11:57 ` [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain() Matthieu Moy
4 siblings, 0 replies; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 11:57 UTC (permalink / raw)
To: git, gitster; +Cc: Jim Meyering, Matthieu Moy
This is a preparation patch to let setup_unpack_trees_porcelain set
show_all_errors itself.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
builtin/checkout.c | 2 +-
builtin/merge.c | 2 +-
merge-recursive.c | 2 +-
unpack-trees.c | 4 +++-
unpack-trees.h | 3 ++-
5 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index e5c0ef0..b26dfd0 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -376,7 +376,7 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.src_index = &the_index;
topts.dst_index = &the_index;
- setup_unpack_trees_porcelain(topts.msgs, "checkout");
+ setup_unpack_trees_porcelain(&topts, "checkout");
refresh_cache(REFRESH_QUIET);
diff --git a/builtin/merge.c b/builtin/merge.c
index da52b10..389e79c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -706,7 +706,7 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
opts.merge = 1;
opts.fn = twoway_merge;
opts.show_all_errors = 1;
- setup_unpack_trees_porcelain(opts.msgs, "merge");
+ setup_unpack_trees_porcelain(&opts, "merge");
trees[nr_trees] = parse_tree_indirect(head);
if (!trees[nr_trees++])
diff --git a/merge-recursive.c b/merge-recursive.c
index 61e237b..ebe6700 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -180,7 +180,7 @@ static int git_merge_trees(int index_only,
opts.fn = threeway_merge;
opts.src_index = &the_index;
opts.dst_index = &the_index;
- setup_unpack_trees_porcelain(opts.msgs, "merge");
+ setup_unpack_trees_porcelain(&opts, "merge");
init_tree_desc_from_tree(t+0, common);
init_tree_desc_from_tree(t+1, head);
diff --git a/unpack-trees.c b/unpack-trees.c
index 4520aa0..17501d3 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -50,8 +50,10 @@ const char *unpack_plumbing_errors[NB_UNPACK_TREES_ERROR_TYPES] = {
? ((o)->msgs[(type)]) \
: (unpack_plumbing_errors[(type)]) )
-void setup_unpack_trees_porcelain(const char **msgs, const char *cmd)
+void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
+ const char *cmd)
{
+ const char **msgs = opts->msgs;
const char *msg;
char *tmp;
const char *cmd2 = strcmp(cmd, "checkout") ? cmd : "switch branches";
diff --git a/unpack-trees.h b/unpack-trees.h
index d62bba9..fad680d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -26,7 +26,8 @@ enum unpack_trees_error_types {
* Sets the list of user-friendly error messages to be used by the
* command "cmd" (either merge or checkout)
*/
-void setup_unpack_trees_porcelain(const char **msgs, const char *cmd);
+void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
+ const char *cmd);
struct rejected_paths_list {
char *path;
--
1.7.2.2.175.ga619d.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
2010-09-01 17:04 ` Matthieu Moy
` (3 preceding siblings ...)
2010-09-02 11:57 ` [PATCH 2/3] setup_unpack_trees_porcelain: take the whole options struct as parameter Matthieu Moy
@ 2010-09-02 11:57 ` Matthieu Moy
2010-09-02 15:52 ` Junio C Hamano
4 siblings, 1 reply; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 11:57 UTC (permalink / raw)
To: git, gitster; +Cc: Jim Meyering, Matthieu Moy
Not only this makes the code clearer since setting up the porcelain error
message is meant to work with show_all_errors, but this fixes a call to
setup_unpack_trees_porcelain() in git_merge_trees() which did not set
show_all_errors.
add_rejected_path() used to double-check whether it was running in
plumbing mode. This check was inefficient since it was setting
show_all_errors too late for traverse_trees() to see it, and is made
useless by this patch. Remove it.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
builtin/checkout.c | 1 -
builtin/merge.c | 1 -
unpack-trees.c | 8 ++------
unpack-trees.h | 2 +-
4 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b26dfd0..1532669 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -395,7 +395,6 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.dir = xcalloc(1, sizeof(*topts.dir));
topts.dir->flags |= DIR_SHOW_IGNORED;
topts.dir->exclude_per_dir = ".gitignore";
- topts.show_all_errors = 1;
tree = parse_tree_indirect(old->commit ?
old->commit->object.sha1 :
(unsigned char *)EMPTY_TREE_SHA1_BIN);
diff --git a/builtin/merge.c b/builtin/merge.c
index 389e79c..bfd3d32 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -705,7 +705,6 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
opts.verbose_update = 1;
opts.merge = 1;
opts.fn = twoway_merge;
- opts.show_all_errors = 1;
setup_unpack_trees_porcelain(&opts, "merge");
trees[nr_trees] = parse_tree_indirect(head);
diff --git a/unpack-trees.c b/unpack-trees.c
index 17501d3..803445a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -94,6 +94,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
"The following Working tree files would be overwritten by sparse checkout update:\n%s";
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
"The following Working tree files would be removed by sparse checkout update:\n%s";
+
+ opts->show_all_errors = 1;
}
static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
@@ -123,12 +125,6 @@ static int add_rejected_path(struct unpack_trees_options *o,
const char *path)
{
struct rejected_paths_list *newentry;
- int porcelain = o && (o)->msgs[e];
- /*
- * simply display the given error message if in plumbing mode
- */
- if (!porcelain)
- o->show_all_errors = 0;
if (!o->show_all_errors)
return error(ERRORMSG(o, e), path);
diff --git a/unpack-trees.h b/unpack-trees.h
index fad680d..7c0187d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -24,7 +24,7 @@ enum unpack_trees_error_types {
/*
* Sets the list of user-friendly error messages to be used by the
- * command "cmd" (either merge or checkout)
+ * command "cmd" (either merge or checkout), and show_all_errors to 1.
*/
void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
const char *cmd);
--
1.7.2.2.175.ga619d.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
2010-09-02 11:57 ` [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain() Matthieu Moy
@ 2010-09-02 15:52 ` Junio C Hamano
2010-09-02 16:06 ` Matthieu Moy
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2010-09-02 15:52 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Jim Meyering
Matthieu Moy <Matthieu.Moy@imag.fr> writes:
> Not only this makes the code clearer since setting up the porcelain error
> message is meant to work with show_all_errors, but this fixes a call to
> setup_unpack_trees_porcelain() in git_merge_trees() which did not set
> show_all_errors.
>
> add_rejected_path() used to double-check whether it was running in
> plumbing mode. This check was inefficient since it was setting
> show_all_errors too late for traverse_trees() to see it, and is made
> useless by this patch. Remove it.
Do you mean inefficient or ineffective?
The code in this patch looks fine, though.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
2010-09-02 15:52 ` Junio C Hamano
@ 2010-09-02 16:06 ` Matthieu Moy
2010-09-02 16:08 ` [PATCH v2] " Matthieu Moy
0 siblings, 1 reply; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 16:06 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jim Meyering
Junio C Hamano <gitster@pobox.com> writes:
> Matthieu Moy <Matthieu.Moy@imag.fr> writes:
>
>> Not only this makes the code clearer since setting up the porcelain error
>> message is meant to work with show_all_errors, but this fixes a call to
>> setup_unpack_trees_porcelain() in git_merge_trees() which did not set
>> show_all_errors.
>>
>> add_rejected_path() used to double-check whether it was running in
>> plumbing mode. This check was inefficient since it was setting
>> show_all_errors too late for traverse_trees() to see it, and is made
>> useless by this patch. Remove it.
>
> Do you mean inefficient or ineffective?
Ineffective, yes.
<disgression>
I should have listened better during my english courses ;-). Both
words translate to "inefficace" in French, and it's a common mistake
for french people.
</disgression>
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] Move "show_all_errors = 1" to setup_unpack_trees_porcelain()
2010-09-02 16:06 ` Matthieu Moy
@ 2010-09-02 16:08 ` Matthieu Moy
0 siblings, 0 replies; 18+ messages in thread
From: Matthieu Moy @ 2010-09-02 16:08 UTC (permalink / raw)
To: git, gitster; +Cc: Matthieu Moy
Not only this makes the code clearer since setting up the porcelain error
message is meant to work with show_all_errors, but this fixes a call to
setup_unpack_trees_porcelain() in git_merge_trees() which did not set
show_all_errors.
add_rejected_path() used to double-check whether it was running in
plumbing mode. This check was ineffective since it was setting
show_all_errors too late for traverse_trees() to see it, and is made
useless by this patch. Remove it.
Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
Just s/inefficient/ineffective/ compared to v1
builtin/checkout.c | 1 -
builtin/merge.c | 1 -
unpack-trees.c | 8 ++------
unpack-trees.h | 2 +-
4 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/builtin/checkout.c b/builtin/checkout.c
index b26dfd0..1532669 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -395,7 +395,6 @@ static int merge_working_tree(struct checkout_opts *opts,
topts.dir = xcalloc(1, sizeof(*topts.dir));
topts.dir->flags |= DIR_SHOW_IGNORED;
topts.dir->exclude_per_dir = ".gitignore";
- topts.show_all_errors = 1;
tree = parse_tree_indirect(old->commit ?
old->commit->object.sha1 :
(unsigned char *)EMPTY_TREE_SHA1_BIN);
diff --git a/builtin/merge.c b/builtin/merge.c
index 389e79c..bfd3d32 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -705,7 +705,6 @@ int checkout_fast_forward(const unsigned char *head, const unsigned char *remote
opts.verbose_update = 1;
opts.merge = 1;
opts.fn = twoway_merge;
- opts.show_all_errors = 1;
setup_unpack_trees_porcelain(&opts, "merge");
trees[nr_trees] = parse_tree_indirect(head);
diff --git a/unpack-trees.c b/unpack-trees.c
index 17501d3..803445a 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -94,6 +94,8 @@ void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
"The following Working tree files would be overwritten by sparse checkout update:\n%s";
msgs[ERROR_WOULD_LOSE_ORPHANED_REMOVED] =
"The following Working tree files would be removed by sparse checkout update:\n%s";
+
+ opts->show_all_errors = 1;
}
static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
@@ -123,12 +125,6 @@ static int add_rejected_path(struct unpack_trees_options *o,
const char *path)
{
struct rejected_paths_list *newentry;
- int porcelain = o && (o)->msgs[e];
- /*
- * simply display the given error message if in plumbing mode
- */
- if (!porcelain)
- o->show_all_errors = 0;
if (!o->show_all_errors)
return error(ERRORMSG(o, e), path);
diff --git a/unpack-trees.h b/unpack-trees.h
index fad680d..7c0187d 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -24,7 +24,7 @@ enum unpack_trees_error_types {
/*
* Sets the list of user-friendly error messages to be used by the
- * command "cmd" (either merge or checkout)
+ * command "cmd" (either merge or checkout), and show_all_errors to 1.
*/
void setup_unpack_trees_porcelain(struct unpack_trees_options *opts,
const char *cmd);
--
1.7.2.2.175.ga619d.dirty
^ permalink raw reply related [flat|nested] 18+ messages in thread