threads / patch / 15776

patchFix argument handling for fetch-pack call when stdout is connected and -q/--quiet is supplied.

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

## tl;dr

6 messages between Oct 3, 2008 and Oct 3, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Tuncer Ayaz· Oct 3, 2008, 19:34 UTC · lore

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(-)
Show changes to transport.c +1 −1
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
Daniel Barkalow· Oct 3, 2008, 19:50 UTC · re: Tuncer Ayaz · lore

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

On Fri, 3 Oct 2008, Tuncer Ayaz wrote:
Show 31 quoted lines
> 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.

Show 6 quoted lines
>     args.depth = data->depth;
> 
>     for (i = 0; i < nr_heads; i++)
> -- 
> 1.6.0.2
>
Tuncer Ayaz· Oct 3, 2008, 20:18 UTC · re: Daniel Barkalow · lore

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

On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow <barkalow@iabervon.org> wrote:
> On Fri, 3 Oct 2008, Tuncer Ayaz wrote:
<snip>
Show 15 quoted lines
>> 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(-)
Show changes to transport.c +2 −2
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
Constantine Plotnikov· Oct 3, 2008, 20:33 UTC · re: Tuncer Ayaz · lore

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

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:
Show 52 quoted lines
> 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
>
Daniel Barkalow· Oct 3, 2008, 20:39 UTC · re: Constantine Plotnikov · lore

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

On Sat, 4 Oct 2008, Constantine Plotnikov wrote:
Show 5 quoted lines
> 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*
Tuncer Ayaz· Oct 3, 2008, 20:44 UTC · re: Constantine Plotnikov · lore

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

On Fri, Oct 3, 2008 at 10:33 PM, Constantine Plotnikov <constantine.plotnikov@gmail.com> wrote:

Show 5 quoted lines
> 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.
Show 58 quoted lines
> 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
>>
>

← back to recent threads