Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
I don't think we ever merged this patch. Any arguments against it? Else I'll go ahead and merge it in a day or two. On Mon, Mar 16, 2015 at 2:47 AM, Giuseppe Scrivano gscriv...@gnu.org wrote: Miquel Llobet mllobet...@gmail.com writes: Yeah, I'll be using git format patch from now on. Do you prefer pasting the commit on the message or attaching a file? shouldn't make a difference, but just attach it, less risky to be changed by the mail client. Thanks, Giuseppe -- Thanking You, Darshit Shah
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
On Sat, Aug 15, 2015 at 7:25 PM, Ander Juaristi ajuari...@gmx.es wrote: On 08/15/2015 09:30 AM, Darshit Shah wrote: I don't think we ever merged this patch. Any arguments against it? Else I'll go ahead and merge it in a day or two. ACK from me. Now that someone has dust off this, you can also merge #40426, which is somewhat related. It looks like the definite solution for now is not to allow -O- and -r together. The two patches provided go for that same way. I would merge the first one proposed by Daniele Calore on Nov 11, 2013. It's more complete, in my opinion. Aye! I pushed both patches On Mon, Mar 16, 2015 at 2:47 AM, Giuseppe Scrivano gscriv...@gnu.org wrote: Miquel Llobet mllobet...@gmail.com writes: Yeah, I'll be using git format patch from now on. Do you prefer pasting the commit on the message or attaching a file? shouldn't make a difference, but just attach it, less risky to be changed by the mail client. Thanks, Giuseppe -- Regards, - AJ -- Thanking You, Darshit Shah
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
On 08/15/2015 09:30 AM, Darshit Shah wrote: I don't think we ever merged this patch. Any arguments against it? Else I'll go ahead and merge it in a day or two. ACK from me. Now that someone has dust off this, you can also merge #40426, which is somewhat related. It looks like the definite solution for now is not to allow -O- and -r together. The two patches provided go for that same way. I would merge the first one proposed by Daniele Calore on Nov 11, 2013. It's more complete, in my opinion. On Mon, Mar 16, 2015 at 2:47 AM, Giuseppe Scrivano gscriv...@gnu.org wrote: Miquel Llobet mllobet...@gmail.com writes: Yeah, I'll be using git format patch from now on. Do you prefer pasting the commit on the message or attaching a file? shouldn't make a difference, but just attach it, less risky to be changed by the mail client. Thanks, Giuseppe -- Regards, - AJ
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
Miquel Llobet mllobet...@gmail.com writes:
It seems like the correct thing to do. Could you please just use
as
ChangeLog-style like commit message?
Yes! Sorry about that, here is the patch with the new commit message.
Let me know if this is correct.
Fixed #44516 -o- not logging to stdout
src/log.c (log_init): check for hypen on filename, set stdout
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror (errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = stdout;
+ }
+ else
+ {
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+ {
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror (errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+ }
}
}
I think the indentation got screwed here. Would you send the patch
attaching what 'git format-patch' gives you or using git send-email?
As the patch doesn't change many lines, we can merge it before waiting
for the copyright assignments to the FSF.
Thanks,
Giuseppe
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
Yeah, I'll be using git format patch from now on. Do you prefer pasting the
commit on the message or attaching a file?
Best,
Miquel Llobet
On Sun, Mar 15, 2015 at 8:59 PM, Giuseppe Scrivano gscriv...@gnu.org
wrote:
Miquel Llobet mllobet...@gmail.com writes:
It seems like the correct thing to do. Could you please just use
as
ChangeLog-style like commit message?
Yes! Sorry about that, here is the patch with the new commit message.
Let me know if this is correct.
Fixed #44516 -o- not logging to stdout
src/log.c (log_init): check for hypen on filename, set stdout
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror (errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = stdout;
+ }
+ else
+ {
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+ {
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror (errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+ }
}
}
I think the indentation got screwed here. Would you send the patch
attaching what 'git format-patch' gives you or using git send-email?
As the patch doesn't change many lines, we can merge it before waiting
for the copyright assignments to the FSF.
Thanks,
Giuseppe
0001-Fixed-44516-o-not-logging-to-stdout.patch
Description: Binary data
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
Miquel Llobet mllobet...@gmail.com writes: Yeah, I'll be using git format patch from now on. Do you prefer pasting the commit on the message or attaching a file? shouldn't make a difference, but just attach it, less risky to be changed by the mail client. Thanks, Giuseppe
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
However, I must say, I support your first version more. It's okay that
currently there is only one statement in that if condition. I believe it's
a lot more clearer with the braces around.
I think so too, normally I write if statements with braces all the time;
will keep it that way :-).
I just had another question. When -o- is passed to Wget, should the
progress bar still be printed to stderr? If not, we need to take care of
that condition too.
Good point, you mean when using --show-progress? I'm not sure, because when
doing -o file --show-progress, then you log to a file but see the
progress bar on stderr? Anyhow the change is very simple:
in src/log.c change get_progress_fp to return get_log_fp(); no matter what.
What do you think?
Miquel Llobet
On Sat, Mar 14, 2015 at 8:10 PM, Darshit Shah dar...@gmail.com wrote:
I just had another question. When -o- is passed to Wget, should the
progress bar still be printed to stderr? If not, we need to take care of
that condition too.
On 03/15, Darshit Shah wrote:
Another good patch Miquel.
However, I must say, I support your first version more. It's okay that
currently there is only one statement in that if condition. I believe it's
a lot more clearer with the braces around.
On 03/13, Miquel Llobet wrote:
removed braces from the second if statement, as per GNU's coding
standards
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 02:28:25.0 +0100
@@ -598,11 +598,16 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
+logfp = stdout;
+ else
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
On Fri, Mar 13, 2015 at 2:04 AM, Miquel Llobet mllobet...@gmail.com
wrote:
wget now correctly reads that -o- means logging to stdout instead of the
file '-'.
I just checked for a hyphen at log_init, didn't see any caveats to this.
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+logfp = stdout;
+}
+ else
+{
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file,
strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
--- end quoted text ---
--
Thanking You,
Darshit Shah
--- end quoted text ---
--
Thanking You,
Darshit Shah
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
I just had another question. When -o- is passed to Wget, should the progress bar
still be printed to stderr? If not, we need to take care of that condition too.
On 03/15, Darshit Shah wrote:
Another good patch Miquel.
However, I must say, I support your first version more. It's okay that
currently there is only one statement in that if condition. I believe
it's a lot more clearer with the braces around.
On 03/13, Miquel Llobet wrote:
removed braces from the second if statement, as per GNU's coding standards
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 02:28:25.0 +0100
@@ -598,11 +598,16 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
+logfp = stdout;
+ else
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
On Fri, Mar 13, 2015 at 2:04 AM, Miquel Llobet mllobet...@gmail.com wrote:
wget now correctly reads that -o- means logging to stdout instead of the
file '-'.
I just checked for a hyphen at log_init, didn't see any caveats to this.
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+logfp = stdout;
+}
+ else
+{
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
--- end quoted text ---
--
Thanking You,
Darshit Shah
--- end quoted text ---
--
Thanking You,
Darshit Shah
pgptwskwjElTv.pgp
Description: PGP signature
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
It seems like the correct thing to do. Could you please just use as
ChangeLog-style like commit message?
Yes! Sorry about that, here is the patch with the new commit message. Let
me know if this is correct.
Fixed #44516 -o- not logging to stdout
src/log.c (log_init): check for hypen on filename, set stdout
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+logfp = stdout;
+}
+ else
+{
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
On Sat, Mar 14, 2015 at 1:19 PM, Giuseppe Scrivano gscriv...@gnu.org
wrote:
Miquel Llobet mllobet...@gmail.com writes:
removed braces from the second if statement, as per GNU's coding
standards
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 02:28:25.0 +0100
@@ -598,11 +598,16 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
+logfp = stdout;
+ else
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
It seems like the correct thing to do. Could you please just use as
ChangeLog-style like commit message?
Thanks,
Giuseppe
Re: [Bug-wget] [Patch] fix bug #44516, -o- log to stdout
Miquel Llobet mllobet...@gmail.com writes:
removed braces from the second if statement, as per GNU's coding standards
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 02:28:25.0 +0100
@@ -598,11 +598,16 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
+logfp = stdout;
+ else
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
It seems like the correct thing to do. Could you please just use as
ChangeLog-style like commit message?
Thanks,
Giuseppe
[Bug-wget] [Patch] fix bug #44516, -o- log to stdout
wget now correctly reads that -o- means logging to stdout instead of the
file '-'.
I just checked for a hyphen at log_init, didn't see any caveats to this.
--- src/log.c.origin 2015-03-13 01:32:27.0 +0100
+++ src/log.c 2015-03-13 01:44:31.0 +0100
@@ -598,11 +598,18 @@
{
if (file)
{
- logfp = fopen (file, appendp ? a : w);
- if (!logfp)
+ if (HYPHENP (file))
{
- fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
- exit (WGET_EXIT_GENERIC_ERROR);
+logfp = stdout;
+}
+ else
+{
+ logfp = fopen (file, appendp ? a : w);
+ if (!logfp)
+{
+ fprintf (stderr, %s: %s: %s\n, exec_name, file, strerror
(errno));
+ exit (WGET_EXIT_GENERIC_ERROR);
+}
}
}
else
Miquel Llobet
