[PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Tuncer Ayaz]

Following is a patch to complete the changes discussed
here http://marc.info/?l=git&m=121529226023180&w=2.

I hope it makes sense and doesn't break something else.

With this simple one-liner patch applied I no longer see
the following remote messages as no-progress is correctly
sent to the remote site:
remote: Counting objects: 84102, done.
remote: Compressing objects: 100% (24720/24720), done.
remote: Total 84102 (delta 60949), reused 80810 (delta 57900)

Regards,

        Tuncer Ayaz

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 transport.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport.c b/transport.c
index 71433d9..1f24011 100644
--- a/transport.c
+++ b/transport.c
@@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
    args.include_tag = data->followtags;
    args.verbose = (transport->verbose > 0);
    args.quiet = args.no_progress = (transport->verbose < 0);
-   args.no_progress = !isatty(1);
+   args.no_progress = args.quiet || !isatty(1);
    args.depth = data->depth;

    for (i = 0; i < nr_heads; i++)
-- 
1.6.0.2

Re: [PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Daniel Barkalow]

On Fri, 3 Oct 2008, Tuncer Ayaz wrote:

> Following is a patch to complete the changes discussed
> here http://marc.info/?l=git&m=121529226023180&w=2.
> 
> I hope it makes sense and doesn't break something else.
> 
> With this simple one-liner patch applied I no longer see
> the following remote messages as no-progress is correctly
> sent to the remote site:
> remote: Counting objects: 84102, done.
> remote: Compressing objects: 100% (24720/24720), done.
> remote: Total 84102 (delta 60949), reused 80810 (delta 57900)
> 
> Regards,
> 
>         Tuncer Ayaz
> 
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
>  transport.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/transport.c b/transport.c
> index 71433d9..1f24011 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>     args.include_tag = data->followtags;
>     args.verbose = (transport->verbose > 0);
>     args.quiet = args.no_progress = (transport->verbose < 0);
> -   args.no_progress = !isatty(1);
> +   args.no_progress = args.quiet || !isatty(1);

If you're doing that, remove the "args.no_progress =" from the previous 
line, which was there to have that effect (but not so clearly). Aside from 
that, it looks good to me.

>     args.depth = data->depth;
> 
>     for (i = 0; i < nr_heads; i++)
> -- 
> 1.6.0.2
> 

Re: [PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Tuncer Ayaz]

On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 3 Oct 2008, Tuncer Ayaz wrote:

<snip>

>> diff --git a/transport.c b/transport.c
>> index 71433d9..1f24011 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>>     args.include_tag = data->followtags;
>>     args.verbose = (transport->verbose > 0);
>>     args.quiet = args.no_progress = (transport->verbose < 0);
>> -   args.no_progress = !isatty(1);
>> +   args.no_progress = args.quiet || !isatty(1);
>
> If you're doing that, remove the "args.no_progress =" from the previous
> line, which was there to have that effect (but not so clearly). Aside from
> that, it looks good to me.
>

<snip>

Thanks Daniel, that makes a lot of sense.

Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
---
 transport.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/transport.c b/transport.c
index 71433d9..35cac44 100644
--- a/transport.c
+++ b/transport.c
@@ -643,8 +643,8 @@ static int fetch_refs_via_pack(struct transport *transport,
        args.use_thin_pack = data->thin;
        args.include_tag = data->followtags;
        args.verbose = (transport->verbose > 0);
-       args.quiet = args.no_progress = (transport->verbose < 0);
-       args.no_progress = !isatty(1);
+       args.quiet = (transport->verbose < 0);
+       args.no_progress = args.quiet || !isatty(1);
        args.depth = data->depth;

        for (i = 0; i < nr_heads; i++)
-- 
1.6.0.2

Re: [PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Constantine Plotnikov]

I think such patch would be too harsh for IDE plugin developers. When
git is run from IDE, the isatty(1) will be true. But progress
information will be still useful, to display an operation progress to
user. Please provide a way to force output of progress (for example
using environment).

Regards,
Constantine


On Sat, Oct 4, 2008 at 12:18 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
> On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
>> On Fri, 3 Oct 2008, Tuncer Ayaz wrote:
>
> <snip>
>
>>> diff --git a/transport.c b/transport.c
>>> index 71433d9..1f24011 100644
>>> --- a/transport.c
>>> +++ b/transport.c
>>> @@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>>>     args.include_tag = data->followtags;
>>>     args.verbose = (transport->verbose > 0);
>>>     args.quiet = args.no_progress = (transport->verbose < 0);
>>> -   args.no_progress = !isatty(1);
>>> +   args.no_progress = args.quiet || !isatty(1);
>>
>> If you're doing that, remove the "args.no_progress =" from the previous
>> line, which was there to have that effect (but not so clearly). Aside from
>> that, it looks good to me.
>>
>
> <snip>
>
> Thanks Daniel, that makes a lot of sense.
>
> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
> ---
>  transport.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 71433d9..35cac44 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -643,8 +643,8 @@ static int fetch_refs_via_pack(struct transport *transport,
>        args.use_thin_pack = data->thin;
>        args.include_tag = data->followtags;
>        args.verbose = (transport->verbose > 0);
> -       args.quiet = args.no_progress = (transport->verbose < 0);
> -       args.no_progress = !isatty(1);
> +       args.quiet = (transport->verbose < 0);
> +       args.no_progress = args.quiet || !isatty(1);
>        args.depth = data->depth;
>
>        for (i = 0; i < nr_heads; i++)
> --
> 1.6.0.2
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Daniel Barkalow]

On Sat, 4 Oct 2008, Constantine Plotnikov wrote:

> I think such patch would be too harsh for IDE plugin developers. When
> git is run from IDE, the isatty(1) will be true. But progress
> information will be still useful, to display an operation progress to
> user. Please provide a way to force output of progress (for example
> using environment).

Probably progress.h ought to have something for whether progress bars make 
sense, that uses isatty(1) with an environment variable override. 
Conceptually, transport.c should be figuring out whether the verbosity of 
the operation suggests progress bars, and should pass off to something 
else the determination of whether we can actually output progress bars in 
particular.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH] Fix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

[Tuncer Ayaz]

On Fri, Oct 3, 2008 at 10:33 PM, Constantine Plotnikov
<constantine.plotnikov@gmail.com> wrote:
> I think such patch would be too harsh for IDE plugin developers. When
> git is run from IDE, the isatty(1) will be true. But progress
> information will be still useful, to display an operation progress to
> user. Please provide a way to force output of progress (for example
> using environment).

Hi Constantine,

this only fixes what was wrong: If you call "git clone -q" args.no_progress
was meant to be 1 but wasn't due to a small error.
If isatty(1) returns 1 then args.no_progress=0, so you will get progress
logging in that case - assuming you haven't supplied -q.

To get progress logging call without -q _and_ have isatty(1)==1.

> Regards,
> Constantine
>
>
> On Sat, Oct 4, 2008 at 12:18 AM, Tuncer Ayaz <tuncer.ayaz@gmail.com> wrote:
>> On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
>>> On Fri, 3 Oct 2008, Tuncer Ayaz wrote:
>>
>> <snip>
>>
>>>> diff --git a/transport.c b/transport.c
>>>> index 71433d9..1f24011 100644
>>>> --- a/transport.c
>>>> +++ b/transport.c
>>>> @@ -644,7 +644,7 @@ static int fetch_refs_via_pack(struct transport *transport,
>>>>     args.include_tag = data->followtags;
>>>>     args.verbose = (transport->verbose > 0);
>>>>     args.quiet = args.no_progress = (transport->verbose < 0);
>>>> -   args.no_progress = !isatty(1);
>>>> +   args.no_progress = args.quiet || !isatty(1);
>>>
>>> If you're doing that, remove the "args.no_progress =" from the previous
>>> line, which was there to have that effect (but not so clearly). Aside from
>>> that, it looks good to me.
>>>
>>
>> <snip>
>>
>> Thanks Daniel, that makes a lot of sense.
>>
>> Signed-off-by: Tuncer Ayaz <tuncer.ayaz@gmail.com>
>> ---
>>  transport.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/transport.c b/transport.c
>> index 71433d9..35cac44 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -643,8 +643,8 @@ static int fetch_refs_via_pack(struct transport *transport,
>>        args.use_thin_pack = data->thin;
>>        args.include_tag = data->followtags;
>>        args.verbose = (transport->verbose > 0);
>> -       args.quiet = args.no_progress = (transport->verbose < 0);
>> -       args.no_progress = !isatty(1);
>> +       args.quiet = (transport->verbose < 0);
>> +       args.no_progress = args.quiet || !isatty(1);
>>        args.depth = data->depth;
>>
>>        for (i = 0; i < nr_heads; i++)
>> --
>> 1.6.0.2
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>