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

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 <redacted>
---
 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

On Fri, 3 Oct 2008, Tuncer Ayaz wrote:

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

On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow [off-list ref] wrote:

On Fri, 3 Oct 2008, Tuncer Ayaz wrote:

<snip>

quoted

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 <redacted>
---
 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

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 [off-list ref] wrote:

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*

On Fri, Oct 3, 2008 at 10:33 PM, Constantine Plotnikov
[off-list ref] 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 [off-list ref] wrote:

quoted

On Fri, Oct 3, 2008 at 9:50 PM, Daniel Barkalow [off-list ref] wrote:

quoted

On Fri, 3 Oct 2008, Tuncer Ayaz wrote:

<snip>

quoted

quoted

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 <redacted>
---
 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