@matheusbsbs / matheus-bagliano / commits / 8ccba008ee
Fork 0

unpack-trees: allow Porcelain to give different error messages

The plumbing output is sacred as it is an API. We _could_ change it if it is broken in such a way that it cannot convey necessary information fully, but we just do not _reword_ for the sake of rewording. If somebody does not like it, s/he is complaining too late. S/he should have been here in early May 2005 and make the language used by the API closer to what humans read. S/he wasn't here. Too bad, and it is too late. And people who complain should look at a bigger picture. Look at what was suggested by one of them and think for five seconds: $ git checkout mytopic -fatal: Entry 'frotz' not uptodate. Cannot merge. +fatal: Entry 'frotz' has local changes. Cannot merge. If you do not see something wrong with this output, your brain has already been rotten with use of git for too long a time. Nobody asked us to "merge" but why are we talking about "Cannot merge"? This patch introduces a mechanism to allow Porcelains to specify messages that are different from the ones that is given by the underlying plumbing implementation of read-tree, so that we can reword the message Porcelains give without disrupting the output from the plumbing. $ git-checkout pu error: You have local changes to 'Makefile'; cannot switch branches. There are other places that ask unpack_trees() to n-way merge, detect issues and let it issue error message on its own, but I did this as a demonstration and replaced only one message. Yes I know about C99 structure initializers. I'd love to use them but we try to be nice to compilers without it. Signed-off-by: Junio C Hamano <gitster@pobox.com>

Junio C Hamano committed May 17, 2008 at 12:03 UTC 8ccba008ee3e0b0582e64cb23ae4ebf734b9f75b
3 files changed +52 -14
builtin-checkout.c
+2
@@ -236,6 +236,8 @@ static int merge_working_tree(struct checkout_opts *opts,
236 topts.src_index = &the_index;
237 topts.dst_index = &the_index;
238
239 + topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches.";
240 +
241 refresh_cache(REFRESH_QUIET);
242
243 if (unmerged_cache()) {
unpack-trees.c
+41 -14
@@ -8,6 +8,36 @@
8 #include "progress.h"
9 #include "refs.h"
10
11 +/*
12 + * Error messages expected by scripts out of plumbing commands such as
13 + * read-tree. Non-scripted Porcelain is not required to use these messages
14 + * and in fact are encouraged to reword them to better suit their particular
15 + * situation better. See how "git checkout" replaces not_uptodate_file to
16 + * explain why it does not allow switching between branches when you have
17 + * local changes, for example.
18 + */
19 +static struct unpack_trees_error_msgs unpack_plumbing_errors = {
20 + /* would_overwrite */
21 + "Entry '%s' would be overwritten by merge. Cannot merge.",
22 +
23 + /* not_uptodate_file */
24 + "Entry '%s' not uptodate. Cannot merge.",
25 +
26 + /* not_uptodate_dir */
27 + "Updating '%s' would lose untracked files in it",
28 +
29 + /* would_lose_untracked */
30 + "Untracked working tree file '%s' would be %s by merge.",
31 +
32 + /* bind_overlap */
33 + "Entry '%s' overlaps with '%s'. Cannot bind.",
34 +};
35 +
36 +#define ERRORMSG(o,fld) \
37 + ( ((o) && (o)->msgs.fld) \
38 + ? ((o)->msgs.fld) \
39 + : (unpack_plumbing_errors.fld) )
40 +
41 static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce,
42 unsigned int set, unsigned int clear)
43 {
@@ -383,10 +413,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
413
414 /* Here come the merge functions */
415
386 -static int reject_merge(struct cache_entry *ce)
416 +static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o)
417 {
388 - return error("Entry '%s' would be overwritten by merge. Cannot merge.",
389 - ce->name);
418 + return error(ERRORMSG(o, would_overwrite), ce->name);
419 }
420
421 static int same(struct cache_entry *a, struct cache_entry *b)
@@ -430,7 +459,7 @@ static int verify_uptodate(struct cache_entry *ce,
459 if (errno == ENOENT)
460 return 0;
461 return o->gently ? -1 :
433 - error("Entry '%s' not uptodate. Cannot merge.", ce->name);
462 + error(ERRORMSG(o, not_uptodate_file), ce->name);
463 }
464
465 static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o)
@@ -517,8 +546,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action,
546 i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL);
547 if (i)
548 return o->gently ? -1 :
520 - error("Updating '%s' would lose untracked files in it",
521 - ce->name);
549 + error(ERRORMSG(o, not_uptodate_dir), ce->name);
550 free(pathbuf);
551 return cnt;
552 }
@@ -618,8 +646,7 @@ static int verify_absent(struct cache_entry *ce, const char *action,
646 }
647
648 return o->gently ? -1 :
621 - error("Untracked working tree file '%s' "
622 - "would be %s by merge.", ce->name, action);
649 + error(ERRORMSG(o, would_lose_untracked), ce->name, action);
650 }
651 return 0;
652 }
@@ -751,7 +778,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)
778 /* #14, #14ALT, #2ALT */
779 if (remote && !df_conflict_head && head_match && !remote_match) {
780 if (index && !same(index, remote) && !same(index, head))
754 - return o->gently ? -1 : reject_merge(index);
781 + return o->gently ? -1 : reject_merge(index, o);
782 return merged_entry(remote, index, o);
783 }
784 /*
@@ -759,7 +786,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o)
786 * make sure that it matches head.
787 */
788 if (index && !same(index, head))
762 - return o->gently ? -1 : reject_merge(index);
789 + return o->gently ? -1 : reject_merge(index, o);
790
791 if (head) {
792 /* #5ALT, #15 */
@@ -901,11 +928,11 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o)
928 else {
929 /* all other failures */
930 if (oldtree)
904 - return o->gently ? -1 : reject_merge(oldtree);
931 + return o->gently ? -1 : reject_merge(oldtree, o);
932 if (current)
906 - return o->gently ? -1 : reject_merge(current);
933 + return o->gently ? -1 : reject_merge(current, o);
934 if (newtree)
908 - return o->gently ? -1 : reject_merge(newtree);
935 + return o->gently ? -1 : reject_merge(newtree, o);
936 return -1;
937 }
938 }
@@ -931,7 +958,7 @@ int bind_merge(struct cache_entry **src,
958 o->merge_size);
959 if (a && old)
960 return o->gently ? -1 :
934 - error("Entry '%s' overlaps with '%s'. Cannot bind.", a->name, old->name);
961 + error(ERRORMSG(o, bind_overlap), a->name, old->name);
962 if (!a)
963 return keep_entry(old, o);
964 else
unpack-trees.h
+9
@@ -8,6 +8,14 @@ struct unpack_trees_options;
8 typedef int (*merge_fn_t)(struct cache_entry **src,
9 struct unpack_trees_options *options);
10
11 +struct unpack_trees_error_msgs {
12 + const char *would_overwrite;
13 + const char *not_uptodate_file;
14 + const char *not_uptodate_dir;
15 + const char *would_lose_untracked;
16 + const char *bind_overlap;
17 +};
18 +
19 struct unpack_trees_options {
20 unsigned int reset:1,
21 merge:1,
@@ -23,6 +31,7 @@ struct unpack_trees_options {
31 int pos;
32 struct dir_struct *dir;
33 merge_fn_t fn;
34 + struct unpack_trees_error_msgs msgs;
35
36 int head_idx;
37 int merge_size;