[PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Tim Harper]

---
 read-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/read-cache.c b/read-cache.c
index a92b25b..971667d 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 changes\n", ce->name);
 			has_errors = 1;
 			continue;
 		}
-- 
1.5.5.1

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Johannes Schindelin]

Hi,

the subject is a nice joke ;-)


On Sat, 3 May 2008, Tim Harper wrote:

> -			printf("%s: needs update\n", ce->name);
> +			printf("%s: has changes\n", ce->name);

How about "local changes"?

Ciao,
Dscho

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Matt Graham]

On Sat, May 3, 2008 at 3:08 AM, Tim Harper <timcharper@gmail.com> wrote:
> ---
>   read-cache.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
>  diff --git a/read-cache.c b/read-cache.c
>  index a92b25b..971667d 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 changes\n", ce->name);
>                         has_errors = 1;
>                         continue;
>                 }
>  --
>  1.5.5.1

Yes, "needs update" is definitely cryptic and confusing.

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Tim Harper]

heh heh - yeah, I used git to send the email and it just sent that  
huge honkin line.

I like "local changes".

There's a bunch of lines that say "not uptodate".  I'll look for them  
all and resend the patch.

Thanks,
Tim

On May 3, 2008, at 8:09 AM, Johannes Schindelin wrote:

> Hi,
>
> the subject is a nice joke ;-)
>
>
> On Sat, 3 May 2008, Tim Harper wrote:
>
>> -			printf("%s: needs update\n", ce->name);
>> +			printf("%s: has changes\n", ce->name);
>
> How about "local changes"?
>
> Ciao,
> Dscho
>

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> On Sat, 3 May 2008, Tim Harper wrote:
>
>> -			printf("%s: needs update\n", ce->name);
>> +			printf("%s: has changes\n", ce->name);
>
> How about "local changes"?

Aren't there Porcelain and end-user scripts that relies on the output by
doing "sed -ne s'/: needs update$//p"?

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Tim Harper]

I ran all of the tests with the patch apply, and they all pass.  Is  
that enough indication?

Tim

On May 3, 2008, at 10:57 AM, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> On Sat, 3 May 2008, Tim Harper wrote:
>>
>>> -			printf("%s: needs update\n", ce->name);
>>> +			printf("%s: has changes\n", ce->name);
>>
>> How about "local changes"?
>
> Aren't there Porcelain and end-user scripts that relies on the  
> output by
> doing "sed -ne s'/: needs update$//p"?
>

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Junio C Hamano]

Tim Harper <timcharper@gmail.com> writes:

because it is very hard to follow the flow of thought.
Please do not top post.

> On May 3, 2008, at 10:57 AM, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> On Sat, 3 May 2008, Tim Harper wrote:
>>>
>>>> -			printf("%s: needs update\n", ce->name);
>>>> +			printf("%s: has changes\n", ce->name);
>>>
>>> How about "local changes"?
>>
>> Aren't there Porcelain and end-user scripts that relies on the
>> output by
>> doing "sed -ne s'/: needs update$//p"?

> I ran all of the tests with the patch apply, and they all pass.  Is
> that enough indication?

Of course not.  Where does end-user scripts come into play when you are
running the testsuite?

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Avery Pennarun]

On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
> > I ran all of the tests with the patch apply, and they all pass.  Is
>  > that enough indication?
>
> Of course not.  Where does end-user scripts come into play when you are
>  running the testsuite?

I thought user scripts weren't supposed to rely on the porcelain
output?  It seems to change rather frequently anyway.

Avery

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Junio C Hamano]

"Avery Pennarun" <apenwarr@gmail.com> writes:

> On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
>> > I ran all of the tests with the patch apply, and they all pass.  Is
>>  > that enough indication?
>>
>> Of course not.  Where does end-user scripts come into play when you are
>>  running the testsuite?
>
> I thought user scripts weren't supposed to rely on the porcelain
> output?  It seems to change rather frequently anyway.

Wasn't the patch about changing output from "update-index --refresh",
which is as low as you can get?

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Johannes Schindelin]

Hi,

On Sat, 3 May 2008, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sat, 3 May 2008, Tim Harper wrote:
> >
> >> -			printf("%s: needs update\n", ce->name);
> >> +			printf("%s: has changes\n", ce->name);
> >
> > How about "local changes"?
> 
> Aren't there Porcelain and end-user scripts that relies on the output by
> doing "sed -ne s'/: needs update$//p"?

Potentially.  But I thought that it would make more sense to use 
--name-only in that case.

However, I obviously like that you go out of your way to cause the least 
damage to current users, so how about something like in merge-recursive, 
where you can change some output based on an environment variable?

In this case, I'd rather make it an option, but that may be overkill.  But 
then, enough people have commented that this message is irritating them.

Ciao,
Dscho

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Avery Pennarun]

On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
> "Avery Pennarun" <apenwarr@gmail.com> writes:
>  > On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
>  >> Of course not.  Where does end-user scripts come into play when you are
>  >>  running the testsuite?
>  >
>  > I thought user scripts weren't supposed to rely on the porcelain
>  > output?  It seems to change rather frequently anyway.
>
> Wasn't the patch about changing output from "update-index --refresh",
>  which is as low as you can get?

Hmm, perhaps the problem then is that we're using plumbing output and
presenting it to the user as part of the porcelain.  Is there an
elegant way to fix that?

Avery

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Jeff King]

On Mon, May 05, 2008 at 12:35:06PM -0400, Avery Pennarun wrote:

> Hmm, perhaps the problem then is that we're using plumbing output and
> presenting it to the user as part of the porcelain.  Is there an
> elegant way to fix that?

2>&1 | sed 's/needs update/has local changes/' ?

Oh wait, you said elegant...

-Peff

Re: [PATCH] I don't known anyone who understands what it means when they do a merge and see "file.txt: needs update". "file.txt: has changes" is much clearer.

[Tim Harper]

Yeah: I was thinking about it earlier and came at the same conclusion.

There's a "porcelain" interface for a lot of commands.  Does the  
concept need to be furthered for this case?

On another note - I've been running with this change for several days,  
and everything seems to be alright.

Tim



On May 5, 2008, at 10:35 AM, Avery Pennarun wrote:

> On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
>> "Avery Pennarun" <apenwarr@gmail.com> writes:
>>> On 5/3/08, Junio C Hamano <gitster@pobox.com> wrote:
>>>> Of course not.  Where does end-user scripts come into play when  
>>>> you are
>>>> running the testsuite?
>>>
>>> I thought user scripts weren't supposed to rely on the porcelain
>>> output?  It seems to change rather frequently anyway.
>>
>> Wasn't the patch about changing output from "update-index --refresh",
>> which is as low as you can get?
>
> Hmm, perhaps the problem then is that we're using plumbing output and
> presenting it to the user as part of the porcelain.  Is there an
> elegant way to fix that?
>
> Avery