Re: [Bug-wget] [PATCH v4] Make wget capable of starting download from a specified position.

2014-02-06 Thread Tim Ruehsen
> > On Thursday 06 February 2014 10:27:37 Yousong Zhou wrote:
> > > On Wednesday, February 5, 2014, Tim Ruehsen  wrote:
> > > > First of all, thanks for your contribution.
> > > > 
> > > > I have some little remarks / questions:
> > > > 
> > > > - The documentation is not quite right: when using --start-pos and the
> > > > file
> > > > already exists, wget creates as expected a file.1.
> > > > But your docs say, --start-pos would overwrite an existing file !?
> > > > Could you make this point clear ?
> > > 
> > > Yes, 'overwrite' is wrong.
> > > 
> > > > - The combination with --continue works for me as expected. It would
> > > > simply
> > > > append the downloaded bytes to the existing file. Maybe you should
> > > > document
> > > > that as well. At least your sentence "... it would override the
> > 
> > behavior
> > 
> > > > of --
> > > > continue" seems not to be correct.
> > > 
> > > Sorry for the confusion.  --continue will detect size of existing file,
> > > then continue as if an equivalent --start-pos was specified.  By
> > 
> > 'override'
> > 
> > > I mean the new option has higher precedence over --continue.  Other than
> > > that, all existing behaviors of wget are supposed to remain unchanged.
> > > 
> > > > - What about extending the option to something like
> > > > --range=STARTPOS[-ENDPOS]
> > > > ?
> > > 
> > > You mean change the option name to 'range'?  IIRC, that's how curl does
> > 
> > it.
> > 
> > >  I am okay with --start-pos. ;)
> > 
> > I just wanted to mention a possible 'ENDPOS'. In that case --start-pos
> > isn't
> > appropriate any more and --range seems natural to me.
> 
> I thought ENDPOS was in the patch and once I did a quick look at it I know
> why I decided it should be --start-pos, without LEN or ENDPOS.
> 
>  - I thought the current implementation is simple, neat and easy.  Several
> lines of code really help at that time.
>  - Code of wget is old and mature enough, mimicing curl's --range is very
> likely to pose many compatibility and maintainance issues.  This is not
> what we want.
>  - I actually thought about LENGTH, but not ENDPOS and the main reason I
> give myself to not implement it is that we can achieve length limit with
> other utilities, e.g. dd.
> 
> So I do not have intention to implement curl-like --range option in wget.

I understand that and agreed with you.
For me, there is just this little documentation issue that I mentioned (here 
is my imperfect suggestion)

Start downloading at zero-based position @var{OFFSET}.  Offset may be 
expressed in bytes, kilobytes with the `k' suffix, or megabytes with the `m' 
suffix.

If combined with @samp{--continue} and the destination file exists, the 
downloaded data will be appended. To avoid appending, you may explicitly 
specify an output filename with @samp{-O FILE}.

Server support for the 'Range' header is needed, otherwise @samp{--start-pos}
cannot help.  See @samp{-c} for details.

Regards, Tim




[Bug-wget] [PATCH v4] Make wget capable of starting download from a specified position.

2014-02-06 Thread Yousong Zhou
On Thursday, February 6, 2014, Tim Ruehsen  wrote:

> Hi Yousong,
>
> please don't forget to send your posts to the mailing list.


Sorry for that.


>
> On Thursday 06 February 2014 10:27:37 Yousong Zhou wrote:
> > On Wednesday, February 5, 2014, Tim Ruehsen  wrote:
> > > First of all, thanks for your contribution.
> > >
> > > I have some little remarks / questions:
> > >
> > > - The documentation is not quite right: when using --start-pos and the
> > > file
> > > already exists, wget creates as expected a file.1.
> > > But your docs say, --start-pos would overwrite an existing file !?
> > > Could you make this point clear ?
> >
> > Yes, 'overwrite' is wrong.
> >
> > > - The combination with --continue works for me as expected. It would
> > > simply
> > > append the downloaded bytes to the existing file. Maybe you should
> > > document
> > > that as well. At least your sentence "... it would override the
> behavior
> > > of --
> > > continue" seems not to be correct.
> >
> > Sorry for the confusion.  --continue will detect size of existing file,
> > then continue as if an equivalent --start-pos was specified.  By
> 'override'
> > I mean the new option has higher precedence over --continue.  Other than
> > that, all existing behaviors of wget are supposed to remain unchanged.
> >
> > > - What about extending the option to something like
> > > --range=STARTPOS[-ENDPOS]
> > > ?
> >
> > You mean change the option name to 'range'?  IIRC, that's how curl does
> it.
> >  I am okay with --start-pos. ;)
>
> I just wanted to mention a possible 'ENDPOS'. In that case --start-pos
> isn't
> appropriate any more and --range seems natural to me.


I thought ENDPOS was in the patch and once I did a quick look at it I know
why I decided it should be --start-pos, without LEN or ENDPOS.

 - I thought the current implementation is simple, neat and easy.  Several
lines of code really help at that time.
 - Code of wget is old and mature enough, mimicing curl's --range is very
likely to pose many compatibility and maintainance issues.  This is not
what we want.
 - I actually thought about LENGTH, but not ENDPOS and the main reason I
give myself to not implement it is that we can achieve length limit with
other utilities, e.g. dd.

So I do not have intention to implement curl-like --range option in wget.


>
> I just took a look at curl's man page. The curl people did it the right
> way.
> Especially their hint about multipart responses is of value (i didn't know
> that). Such cases would likely need special handling in Wget.
>
>
didn't know that either.


> >
> > > - If you want to brush up your patch, add a test-case for it for the
> new
> > > Python based test suite. I guess, Darshit can give you a helping hand,
> if
> > > you
> > > request it.
> >
> > Will do once I get the time.
> >
> > > Tim
> >
> > Thank you for looking at this.
> >
> > yousong
> >
> > > >   --start-pos is zero-based.
> > > >
> > > > v2 -> v3
> > > >
> > > > Fix a typo and add description text for the new option into
> the
> > > >
> > > > usage output.  Thank Darshit Shah >
> for
> > >
> > > the suggestions.
> > >
> > > > v1 -> v2
> > > >
> > > > It was kindly pointed out by Darshit Shah
> > > > >
> > >
> > > that
> > >
> > > > server support for resuming download is required, so adding
> this
> > > >
> > > > into doc/wget.texi.
> > > >
> > > >  doc/ChangeLog |4 
> > > >  doc/wget.texi |   17 +
> > > >  src/ChangeLog |9 +
> > > >  src/ftp.c |2 ++
> > > >  src/http.c|2 ++
> > > >  src/init.c|1 +
> > > >  src/main.c|3 +++
> > > >  src/options.h |1 +
> > > >  8 files changed, 39 insertions(+), 0 deletions(-)
>
>


Re: [Bug-wget] [PATCH v4] Make wget capable of starting download from a specified position.

2014-02-06 Thread Tim Ruehsen
Hi Yousong,

please don't forget to send your posts to the mailing list.

On Thursday 06 February 2014 10:27:37 Yousong Zhou wrote:
> On Wednesday, February 5, 2014, Tim Ruehsen  wrote:
> > First of all, thanks for your contribution.
> > 
> > I have some little remarks / questions:
> > 
> > - The documentation is not quite right: when using --start-pos and the
> > file
> > already exists, wget creates as expected a file.1.
> > But your docs say, --start-pos would overwrite an existing file !?
> > Could you make this point clear ?
> 
> Yes, 'overwrite' is wrong.
> 
> > - The combination with --continue works for me as expected. It would
> > simply
> > append the downloaded bytes to the existing file. Maybe you should
> > document
> > that as well. At least your sentence "... it would override the behavior
> > of --
> > continue" seems not to be correct.
> 
> Sorry for the confusion.  --continue will detect size of existing file,
> then continue as if an equivalent --start-pos was specified.  By 'override'
> I mean the new option has higher precedence over --continue.  Other than
> that, all existing behaviors of wget are supposed to remain unchanged.
> 
> > - What about extending the option to something like
> > --range=STARTPOS[-ENDPOS]
> > ?
> 
> You mean change the option name to 'range'?  IIRC, that's how curl does it.
>  I am okay with --start-pos. ;)

I just wanted to mention a possible 'ENDPOS'. In that case --start-pos isn't 
appropriate any more and --range seems natural to me.

I just took a look at curl's man page. The curl people did it the right way.
Especially their hint about multipart responses is of value (i didn't know 
that). Such cases would likely need special handling in Wget.

> 
> > - If you want to brush up your patch, add a test-case for it for the new
> > Python based test suite. I guess, Darshit can give you a helping hand, if
> > you
> > request it.
> 
> Will do once I get the time.
> 
> > Tim
> 
> Thank you for looking at this.
> 
> yousong
> 
> > >   --start-pos is zero-based.
> > > 
> > > v2 -> v3
> > > 
> > > Fix a typo and add description text for the new option into the
> > > 
> > > usage output.  Thank Darshit Shah > for
> > 
> > the suggestions.
> > 
> > > v1 -> v2
> > > 
> > > It was kindly pointed out by Darshit Shah
> > > >
> > 
> > that
> > 
> > > server support for resuming download is required, so adding this
> > > 
> > > into doc/wget.texi.
> > > 
> > >  doc/ChangeLog |4 
> > >  doc/wget.texi |   17 +
> > >  src/ChangeLog |9 +
> > >  src/ftp.c |2 ++
> > >  src/http.c|2 ++
> > >  src/init.c|1 +
> > >  src/main.c|3 +++
> > >  src/options.h |1 +
> > >  8 files changed, 39 insertions(+), 0 deletions(-)