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