# [PATCH] "not uptodate" changed to "has local changes" 24 messages from 2008-05-03 to 2008-05-21. Participants: Tim Harper, Mike Ralphson, André Goddard Rosa, Johannes Schindelin, Holger Schurig, Francis Moreau, Kevin Ballard, Steven Walter, Wincent Colaiuta, Matthieu Moy, Sverre Rabbelier, Junio C Hamano, Daniel Barkalow. Thread: https://gitlist.dev/t/13367 ## Tim Harper, 2008-05-03 16:59 Subject: [PATCH] "not uptodate" changed to "has local changes" Message-ID: <1209833972-12256-1-git-send-email-timcharper@gmail.com> URL: https://gitlist.dev/e/1209833972-12256-1-git-send-email-timcharper%40gmail.com ``` When doing a merge, the message says "file.txt: needs update", or "file.txt: not uptodate, cannot merge". While internally 'uptodate' makes sense, from the outside it's a mystery. This patch will make git a little more human friendly, reporting "file.txt: has local changes". --- read-cache.c | 2 +- unpack-trees.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/read-cache.c b/read-cache.c index a92b25b..e890b27 100644 --- a/read-cache.c +++ b/read-cache.c @@ -999,7 +999,7 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p } if (quiet) continue; - printf("%s: needs update\n", ce->name); + printf("%s: has local changes\n", ce->name); has_errors = 1; continue; } diff --git a/unpack-trees.c b/unpack-trees.c index a59f475..1d67e08 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -427,7 +427,7 @@ static int verify_uptodate(struct cache_entry *ce, if (errno == ENOENT) return 0; return o->gently ? -1 : - error("Entry '%s' not uptodate. Cannot merge.", ce->name); + error("Entry '%s' has local changes. Cannot merge.", ce->name); } static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o) -- 1.5.5.1 ``` ## Mike Ralphson, 2008-05-06 13:31 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/e2b179460805060631l506e2a6leaafc9c0acf3b05b%40mail.gmail.com In-Reply-To: <1209833972-12256-1-git-send-email-timcharper@gmail.com> ``` 2008/5/3 Tim Harper : > When doing a merge, the message says "file.txt: needs update", or "file.txt: not uptodate, cannot merge". While internally 'uptodate' makes sense, from the outside it's a mystery. > > This patch will make git a little more human friendly, reporting "file.txt: has local changes". Documentation/git-checkout.txt should also change in this case, otherwise users will see different output to that described and possibly get confused if following along with the examples. Mike ``` ## André Goddard Rosa, 2008-05-16 02:14 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/b8bf37780805151914j65ce5406xc5e6b3d29e3bfb9b%40mail.gmail.com In-Reply-To: ``` >> This patch will make git a little more human friendly, reporting "file.txt: has local changes". > > Documentation/git-checkout.txt should also change in this case, > otherwise users will see different output to that described and > possibly get confused if following along with the examples. > I like the idea too. --- [PATCH] "not uptodate" changed to "has local changes" Use more straightforward message for regular user. Signed-off-by: Andre Goddard Rosa diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index a644173..624dea6 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -168,7 +168,7 @@ the above checkout would fail like this: + ------------ $ git checkout mytopic -fatal: Entry 'frotz' not uptodate. Cannot merge. +fatal: Entry 'frotz' has local changes. Cannot merge. ------------ + You can give the `-m` flag to the command, which would try a ``` ## Johannes Schindelin, 2008-05-16 10:25 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0805161125320.30431%40racer In-Reply-To: ``` Hi, On Thu, 15 May 2008, André Goddard Rosa wrote: > >> This patch will make git a little more human friendly, reporting "file.txt: has local changes". > > > > Documentation/git-checkout.txt should also change in this case, > > otherwise users will see different output to that described and > > possibly get confused if following along with the examples. > > > > I like the idea too. No comment on the concern that it might break people's scripts? None? Ciao, Dscho ``` ## Holger Schurig, 2008-05-16 11:02 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: <200805161302.47865.hs4233@mail.mn-solutions.de> URL: https://gitlist.dev/e/200805161302.47865.hs4233%40mail.mn-solutions.de In-Reply-To: ``` > No comment on the concern that it might break people's > scripts? None? Scripts should look for exit values :-) ``` ## Francis Moreau, 2008-05-16 11:50 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: <38b2ab8a0805160450s495aceddy3335d27f076aa64e@mail.gmail.com> URL: https://gitlist.dev/e/38b2ab8a0805160450s495aceddy3335d27f076aa64e%40mail.gmail.com In-Reply-To: <200805161302.47865.hs4233@mail.mn-solutions.de> ``` On Fri, May 16, 2008 at 1:02 PM, Holger Schurig wrote: >> No comment on the concern that it might break people's >> scripts? None? > > Scripts should look for exit values :-) except that they're not documented ;) -- Francis ``` ## Johannes Schindelin, 2008-05-16 12:22 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0805161322090.30431%40racer In-Reply-To: <200805161302.47865.hs4233@mail.mn-solutions.de> ``` Hi, On Fri, 16 May 2008, Holger Schurig wrote: > > No comment on the concern that it might break people's > > scripts? None? > > Scripts should look for exit values :-) Clever. And which exit value would you exactly check to see _which_ file needs an update? Hth, Dscho ``` ## Kevin Ballard, 2008-05-16 17:12 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: <1ED37CF1-EABD-4881-BA29-ED2CB1CE73FC@sb.org> URL: https://gitlist.dev/e/1ED37CF1-EABD-4881-BA29-ED2CB1CE73FC%40sb.org In-Reply-To: ``` On May 16, 2008, at 6:25 AM, Johannes Schindelin wrote: > On Thu, 15 May 2008, André Goddard Rosa wrote: > >>>> This patch will make git a little more human friendly, reporting >>>> "file.txt: has local changes". >>> >>> Documentation/git-checkout.txt should also change in this case, >>> otherwise users will see different output to that described and >>> possibly get confused if following along with the examples. >>> >> >> I like the idea too. > > No comment on the concern that it might break people's scripts? None? How about an ugly hack? Look to see if stdout is a tty, if so spit out the more human-readable version, otherwise spit out the old version >:-) -Kevin -- Kevin Ballard http://kevin.sb.org kevin@sb.org http://www.tildesoft.com ``` ## André Goddard Rosa, 2008-05-17 03:30 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/b8bf37780805162030m4c961505nabd72e8f5bd08404%40mail.gmail.com In-Reply-To: <1ED37CF1-EABD-4881-BA29-ED2CB1CE73FC@sb.org> ``` On Fri, May 16, 2008 at 2:12 PM, Kevin Ballard wrote: > On May 16, 2008, at 6:25 AM, Johannes Schindelin wrote: > >> On Thu, 15 May 2008, André Goddard Rosa wrote: >> >>>>> This patch will make git a little more human friendly, reporting >>>>> "file.txt: has local changes". >>>> >>>> Documentation/git-checkout.txt should also change in this case, >>>> otherwise users will see different output to that described and >>>> possibly get confused if following along with the examples. >>>> >>> >>> I like the idea too. >> >> No comment on the concern that it might break people's scripts? None? > > > How about an ugly hack? Look to see if stdout is a tty, if so spit out the > more human-readable version, otherwise spit out the old version >:-) Is this user interface set on stone? I think we should reserve the right to improve always. I would deprecate the current message, but I think that most users cannot find so much of a sense in the former message, although the script developer can easily change his scripts to search for ´cannot merge´ instead. Do you have a better idea? --- [PATCH] "not uptodate" changed to "has local changes" Use more straightforward message for regular user. Signed-off-by: Andre Goddard Rosa diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index a644173..624dea6 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -168,7 +168,7 @@ the above checkout would fail like this: + ------------ $ git checkout mytopic -fatal: Entry 'frotz' not uptodate. Cannot merge. +fatal: Entry 'frotz' not uptodate, it has local changes. Cannot merge. ------------ + You can give the `-m` flag to the command, which would try a ``` ## Johannes Schindelin, 2008-05-17 10:04 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0805171102480.30431%40racer In-Reply-To: ``` Hi, On Sat, 17 May 2008, André Goddard Rosa wrote: > On Fri, May 16, 2008 at 2:12 PM, Kevin Ballard wrote: > > On May 16, 2008, at 6:25 AM, Johannes Schindelin wrote: > > > >> On Thu, 15 May 2008, André Goddard Rosa wrote: > >> > >>>>> This patch will make git a little more human friendly, reporting > >>>>> "file.txt: has local changes". > >>>> > >>>> Documentation/git-checkout.txt should also change in this case, > >>>> otherwise users will see different output to that described and > >>>> possibly get confused if following along with the examples. > >>>> > >>> > >>> I like the idea too. > >> > >> No comment on the concern that it might break people's scripts? None? > > > > > > How about an ugly hack? Look to see if stdout is a tty, if so spit out the > > more human-readable version, otherwise spit out the old version >:-) > > Is this user interface set on stone? I think we should reserve the right > to improve always. Umm. As has been mentioned, this is not a "user interface". The message you are seeing comes from a _plumbing_ program, i.e. something _not_ meant for human consumption. I still think that it might be better to add a command line option with a custom message, because that would _not_ break backwards-compatibility. Thankyouverymuch, Dscho ``` ## Steven Walter, 2008-05-17 14:44 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/e06498070805170744l7ad213aeqb779b0c0381e1de4%40mail.gmail.com In-Reply-To: ``` On Sat, May 17, 2008 at 6:04 AM, Johannes Schindelin wrote: >> Is this user interface set on stone? I think we should reserve the right >> to improve always. > > Umm. > > As has been mentioned, this is not a "user interface". The message you > are seeing comes from a _plumbing_ program, i.e. something _not_ meant for > human consumption. > > I still think that it might be better to add a command line option with a > custom message, because that would _not_ break backwards-compatibility. With this dedication to backwards-compatibility, we'll be at Windows Vista quality in no time. -- -Steven Walter ``` ## Wincent Colaiuta, 2008-05-17 15:08 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: <393B2C1A-49D3-48A7-BF7D-7938D212AF82@wincent.com> URL: https://gitlist.dev/e/393B2C1A-49D3-48A7-BF7D-7938D212AF82%40wincent.com In-Reply-To: ``` El 17/5/2008, a las 12:04, Johannes Schindelin escribió: > Hi, > > On Sat, 17 May 2008, André Goddard Rosa wrote: > >> Is this user interface set on stone? I think we should reserve the >> right >> to improve always. > > Umm. > > As has been mentioned, this is not a "user interface". The message > you > are seeing comes from a _plumbing_ program, i.e. something _not_ > meant for > human consumption. That would indicate a problem, if stuff not intended for human consumption is being dished up for exactly that: human consumption. > I still think that it might be better to add a command line option > with a > custom message, because that would _not_ break backwards- > compatibility. Sounds like clutter to me. I'd instead favor just holding back this patch until 1.6, when (minor) "compatibility breaking" changes would be acceptable. Wincent ``` ## Matthieu Moy, 2008-05-17 15:32 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/vpq63tdgd5a.fsf%40bauges.imag.fr In-Reply-To: <393B2C1A-49D3-48A7-BF7D-7938D212AF82@wincent.com> ``` Wincent Colaiuta writes: > That would indicate a problem, if stuff not intended for human > consumption is being dished up for exactly that: human consumption. There have been other instances of this one, and the output of git-update-index has been replaced by a call to "git status" in the porcelain. -- Matthieu ``` ## Sverre Rabbelier, 2008-05-17 16:12 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/bd6139dc0805170912w14b7894fxfebe15c14e4c44ef%40mail.gmail.com In-Reply-To: ``` On Sat, May 17, 2008 at 4:44 PM, Steven Walter wrote: > With this dedication to backwards-compatibility, we'll be at Windows > Vista quality in no time. I very much agree here, given the nature of scripts (that is, being very easy to update), I think we should try not to be too strict in backwards-compatibility or we'll lose the flexibility that is very much needed when developing a Good Product (tm) As long as such compatibility breaking changes are marked (in BIG LETTERS) in the changelog/release notes I think that would be a 'sacrifice' we should consider making. -- Cheers, Sverre Rabbelier ``` ## Johannes Schindelin, 2008-05-17 18:44 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0805171939540.30431%40racer In-Reply-To: ``` Hi, On Sat, 17 May 2008, Sverre Rabbelier wrote: > On Sat, May 17, 2008 at 4:44 PM, Steven Walter > wrote: > > > With this dedication to backwards-compatibility, we'll be at Windows > > Vista quality in no time. That is silly at best, especially given that Vista is _not_ backwards-compatible. Not to mention that it is not forkable, because it is not Open Source. > I very much agree here, given the nature of scripts (that is, being very > easy to update), I think we should try not to be too strict in > backwards-compatibility or we'll lose the flexibility that is very much > needed when developing a Good Product (tm) As long as such compatibility > breaking changes are marked (in BIG LETTERS) in the changelog/release > notes I think that would be a 'sacrifice' we should consider making. Had you (one of our GSoc students) not replied, I would not even have bothered to say anything. But I strongly disagree with the notion that it is okay to fsck with old-timers (who would be harmed by breaking backwards-incompatibility, and nobody else), especially given that it is mostly old-timers who turned Git into the Good Product(tm) it is. Ciao, Dscho ``` ## Junio C Hamano, 2008-05-17 19:03 Subject: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: <7v63tcda7e.fsf_-_@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v63tcda7e.fsf_-_%40gitster.siamese.dyndns.org In-Reply-To: ``` "Sverre Rabbelier" writes: > On Sat, May 17, 2008 at 4:44 PM, Steven Walter wrote: >> With this dedication to backwards-compatibility, we'll be at Windows >> Vista quality in no time. > > I very much agree here, given the nature of scripts (that is, being > very easy to update), I think we should try not to be too strict in > backwards-compatibility or we'll lose the flexibility that is very > much needed when developing a Good Product (tm) As long as such > compatibility breaking changes are marked (in BIG LETTERS) in the > changelog/release notes I think that would be a 'sacrifice' we should > consider making. Don't feed the troll by responding to a cheap shot. 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"? Try a different approach along this patch instead. $ 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, which people who complained in this thread can identify and improve, 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. builtin-checkout.c | 2 ++ unpack-trees.c | 47 +++++++++++++++++++++++++++++++++-------------- unpack-trees.h | 9 +++++++++ 3 files changed, 44 insertions(+), 14 deletions(-) diff --git a/builtin-checkout.c b/builtin-checkout.c index 10ec137..83da7ca 100644 --- a/builtin-checkout.c +++ b/builtin-checkout.c @@ -236,6 +236,8 @@ static int merge_working_tree(struct checkout_opts *opts, topts.src_index = &the_index; topts.dst_index = &the_index; + topts.msgs.not_uptodate_file = "You have local changes to '%s'; cannot switch branches."; + refresh_cache(REFRESH_QUIET); if (unmerged_cache()) { diff --git a/unpack-trees.c b/unpack-trees.c index 1ab28fd..bec12dc 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -8,6 +8,28 @@ #include "progress.h" #include "refs.h" +static struct unpack_trees_error_msgs unpack_default_errors = { + /* would_overwrite */ + "Entry '%s' would be overwritten by merge. Cannot merge.", + + /* not_uptodate_file */ + "Entry '%s' not uptodate. Cannot merge.", + + /* not_uptodate_dir */ + "Updating '%s' would lose untracked files in it", + + /* would_lose_untracked */ + "Untracked working tree file '%s' would be %s by merge.", + + /* bind_overlap */ + "Entry '%s' overlaps with '%s'. Cannot bind.", +}; + +#define ERRORMSG(o,fld) \ + ( ((o) && (o)->msgs.fld) \ + ? ((o)->msgs.fld) \ + : (unpack_default_errors.fld) ) + static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce, unsigned int set, unsigned int clear) { @@ -383,10 +405,9 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options /* Here come the merge functions */ -static int reject_merge(struct cache_entry *ce) +static int reject_merge(struct cache_entry *ce, struct unpack_trees_options *o) { - return error("Entry '%s' would be overwritten by merge. Cannot merge.", - ce->name); + return error(ERRORMSG(o, would_overwrite), ce->name); } static int same(struct cache_entry *a, struct cache_entry *b) @@ -430,7 +451,7 @@ static int verify_uptodate(struct cache_entry *ce, if (errno == ENOENT) return 0; return o->gently ? -1 : - error("Entry '%s' not uptodate. Cannot merge.", ce->name); + error(ERRORMSG(o, not_uptodate_file), ce->name); } static void invalidate_ce_path(struct cache_entry *ce, struct unpack_trees_options *o) @@ -517,8 +538,7 @@ static int verify_clean_subdirectory(struct cache_entry *ce, const char *action, i = read_directory(&d, ce->name, pathbuf, namelen+1, NULL); if (i) return o->gently ? -1 : - error("Updating '%s' would lose untracked files in it", - ce->name); + error(ERRORMSG(o, not_uptodate_dir), ce->name); free(pathbuf); return cnt; } @@ -618,8 +638,7 @@ static int verify_absent(struct cache_entry *ce, const char *action, } return o->gently ? -1 : - error("Untracked working tree file '%s' " - "would be %s by merge.", ce->name, action); + error(ERRORMSG(o, would_lose_untracked), ce->name, action); } return 0; } @@ -751,7 +770,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o) /* #14, #14ALT, #2ALT */ if (remote && !df_conflict_head && head_match && !remote_match) { if (index && !same(index, remote) && !same(index, head)) - return o->gently ? -1 : reject_merge(index); + return o->gently ? -1 : reject_merge(index, o); return merged_entry(remote, index, o); } /* @@ -759,7 +778,7 @@ int threeway_merge(struct cache_entry **stages, struct unpack_trees_options *o) * make sure that it matches head. */ if (index && !same(index, head)) - return o->gently ? -1 : reject_merge(index); + return o->gently ? -1 : reject_merge(index, o); if (head) { /* #5ALT, #15 */ @@ -901,11 +920,11 @@ int twoway_merge(struct cache_entry **src, struct unpack_trees_options *o) else { /* all other failures */ if (oldtree) - return o->gently ? -1 : reject_merge(oldtree); + return o->gently ? -1 : reject_merge(oldtree, o); if (current) - return o->gently ? -1 : reject_merge(current); + return o->gently ? -1 : reject_merge(current, o); if (newtree) - return o->gently ? -1 : reject_merge(newtree); + return o->gently ? -1 : reject_merge(newtree, o); return -1; } } @@ -931,7 +950,7 @@ int bind_merge(struct cache_entry **src, o->merge_size); if (a && old) return o->gently ? -1 : - error("Entry '%s' overlaps with '%s'. Cannot bind.", a->name, old->name); + error(ERRORMSG(o, bind_overlap), a->name, old->name); if (!a) return keep_entry(old, o); else diff --git a/unpack-trees.h b/unpack-trees.h index d436d6c..94e5672 100644 --- a/unpack-trees.h +++ b/unpack-trees.h @@ -8,6 +8,14 @@ struct unpack_trees_options; typedef int (*merge_fn_t)(struct cache_entry **src, struct unpack_trees_options *options); +struct unpack_trees_error_msgs { + const char *would_overwrite; + const char *not_uptodate_file; + const char *not_uptodate_dir; + const char *would_lose_untracked; + const char *bind_overlap; +}; + struct unpack_trees_options { unsigned int reset:1, merge:1, @@ -23,6 +31,7 @@ struct unpack_trees_options { int pos; struct dir_struct *dir; merge_fn_t fn; + struct unpack_trees_error_msgs msgs; int head_idx; int merge_size; ``` ## Sverre Rabbelier, 2008-05-17 20:14 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/bd6139dc0805171314i29daf0det47d9ad8f9e7d76a7%40mail.gmail.com In-Reply-To: ``` On Sat, May 17, 2008 at 8:44 PM, Johannes Schindelin > But I strongly disagree with the notion that it is okay to fsck with > old-timers (who would be harmed by breaking backwards-incompatibility, > and nobody else), especially given that it is mostly old-timers who turned > Git into the Good Product(tm) it is. But those old-timers can be updated can't they? Also, shouldn't we have tests to see if they 'break' because of changes? -- Cheers, Sverre Rabbelier ``` ## Sverre Rabbelier, 2008-05-17 20:29 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/bd6139dc0805171329i29b518a2l7b28e7e71756adac%40mail.gmail.com In-Reply-To: <7v63tcda7e.fsf_-_@gitster.siamese.dyndns.org> ``` On Sat, May 17, 2008 at 9:03 PM, Junio C Hamano wrote: > Don't feed the troll by responding to a cheap shot. Yes, a cheap shot, but I think one trying to make a point. > 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. Mhh.. I guess I didn't realize how strongly git still is "many commands that are good at what they do" that together form a coherent entity. > 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"? Very good point, perhaps we should consider double-checking them all and improving them for one of the next 1.x releases. > Try a different approach along this patch instead. > > $ 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, which people who > complained in this thread can identify and improve, but I did this as a > demonstration and replaced only one message. The patch looks like a step in the right direction, but if there is interest in improving the error messaging system why not do it right and make it generic. Instead of each file reinventing the wheel create a more generic system that uses a configuration file (I think this was suggested earlier). If we choose something like that, perhaps it would be nice to add a 'scripting' mode, which provides with a CLI-like messages instead of the usual human-readable ones (and thus less easy to parse). -- Cheers, Sverre Rabbelier ``` ## Sverre Rabbelier, 2008-05-18 11:11 Subject: Re: [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/bd6139dc0805180411k4a2edee3ueef178c141744b35%40mail.gmail.com In-Reply-To: <200805181032.m4IAWjE0012832@mi0.bluebottle.com> ``` On Sun, May 18, 2008 at 12:31 PM, しらいしななこ wrote: > What advantage are you bringing to the table for them to be worth bothering to update, other than "if they update then they can get their scripts working again after you break them by rewording messages for no good reason"? Why do you punish the old-timers for using the well established API? My reason was that (perhaps not as much in this case, but I meant to speak in general) the message is confusing/not clear enough. > Did you study Junio's patch before you responded to his message? The point of the suggestion was that he reworded the message given by git-checkout that is a porcelain command without breaking output from read-tree that is a plumbing command. In other words, you can improve output from porcelain commands without making unnecessary changes to plumbing. I hadn't read his e-mail when I was replying, my Internet has been playing up lately (same for about half of the Netherlands due to a problem at one of our biggest ISPs). I did reply to it though: http://thread.gmane.org/gmane.comp.version-control.git/81100 -- Cheers, Sverre Rabbelier ``` ## Wincent Colaiuta, 2008-05-19 06:55 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/D925ECD3-1EE2-48EB-B1E5-2604C10AD643%40wincent.com In-Reply-To: <7v63tcda7e.fsf_-_@gitster.siamese.dyndns.org> ``` El 17/5/2008, a las 21:03, Junio C Hamano escribió: > + /* not_uptodate_file */ > + "Entry '%s' not uptodate. Cannot merge.", Minor nit, "uptodate" is not a word. Should be either "up-to-date" or "up to date"; most dictionaries list both. Cheers, Wincent ``` ## Junio C Hamano, 2008-05-19 17:47 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: <7vzlqm89tj.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzlqm89tj.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Wincent Colaiuta writes: > El 17/5/2008, a las 21:03, Junio C Hamano escribió: >> + /* not_uptodate_file */ >> + "Entry '%s' not uptodate. Cannot merge.", > > > Minor nit, "uptodate" is not a word. Should be either "up-to-date" or > "up to date"; most dictionaries list both. Why does *everybody* keep missing the whole point of this patch? Grumble. ``` ## Sverre Rabbelier, 2008-05-19 18:28 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/bd6139dc0805191128t13afc8ard051138083e2b694%40mail.gmail.com In-Reply-To: <7vzlqm89tj.fsf@gitster.siamese.dyndns.org> ``` On Mon, May 19, 2008 at 7:47 PM, Junio C Hamano wrote: > Why does *everybody* keep missing the whole point of this patch? Isn't it to make it possible to change the error messages at porcelain level while allowing the plumbing to remain backward compatibility? -- Cheers, Sverre Rabbelier ``` ## Daniel Barkalow, 2008-05-19 19:32 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: URL: https://gitlist.dev/e/alpine.LNX.1.00.0805191523060.19665%40iabervon.org In-Reply-To: <7vzlqm89tj.fsf@gitster.siamese.dyndns.org> ``` On Mon, 19 May 2008, Junio C Hamano wrote: > Wincent Colaiuta writes: > > > El 17/5/2008, a las 21:03, Junio C Hamano escribió: > >> + /* not_uptodate_file */ > >> + "Entry '%s' not uptodate. Cannot merge.", > > > > > > Minor nit, "uptodate" is not a word. Should be either "up-to-date" or > > "up to date"; most dictionaries list both. > > Why does *everybody* keep missing the whole point of this patch? That section needs a comment stating that it's the scripting API, not just an arbitrary set of messages. For that matter, maybe those shouldn't be the default set, but an alternate set used (as a group) by plumbing programs; I don't think it's too likely that there will be a whole lot of new plumbing programs, and new porcelain programs that don't specify anything probably ought to get something more generic. -Daniel *This .sig left intentionally blank* ``` ## Junio C Hamano, 2008-05-21 07:07 Subject: Re: Re* [PATCH] "not uptodate" changed to "has local changes" Message-ID: <7v4p8s6spa.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7v4p8s6spa.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Daniel Barkalow writes: > On Mon, 19 May 2008, Junio C Hamano wrote: > >> Why does *everybody* keep missing the whole point of this patch? > > That section needs a comment stating that it's the scripting API, not just > an arbitrary set of messages. Yeah, that is a very good explanation. Thanks for a constructive suggestion for improvements. Here is an incremental on top of the one I sent out, in case people want to improve on it. unpack-trees.c | 12 ++++++++++-- 1 files changed, 10 insertions(+), 2 deletions(-) diff --git a/unpack-trees.c b/unpack-trees.c index da3bdc8..0de5a31 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -8,7 +8,15 @@ #include "progress.h" #include "refs.h" -static struct unpack_trees_error_msgs unpack_default_errors = { +/* + * Error messages expected by scripts out of plumbing commands such as + * 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" replaces not_uptodate_file to + * explain why it does not allow switching between branches when you have + * local changes, for example. + */ +static struct unpack_trees_error_msgs unpack_plumbing_errors = { /* would_overwrite */ "Entry '%s' would be overwritten by merge. Cannot merge.", @@ -28,7 +36,7 @@ static struct unpack_trees_error_msgs unpack_default_errors = { #define ERRORMSG(o,fld) \ ( ((o) && (o)->msgs.fld) \ ? ((o)->msgs.fld) \ - : (unpack_default_errors.fld) ) + : (unpack_plumbing_errors.fld) ) static void add_entry(struct unpack_trees_options *o, struct cache_entry *ce, unsigned int set, unsigned int clear) ```