On Fri, Jul 11, 2014 at 05:46:02PM +0200, Alexander Hall wrote:
> On 07/11/14 17:35, Lawrence Teo wrote:
> >On Fri, Jul 11, 2014 at 12:20:00PM +0200, Alexander Hall wrote:
> >>On 07/10/14 06:30, Lawrence Teo wrote:
> >>>About a month ago, I sent a diff that allows ftp(1) to set its
> >>>User-Agent.
> >>>
> >>>Based on feedback from halex@ and deraadt@, I have changed it so that
> >>>the User-Agent can be set via a -U command-line option instead of an
> >>>environment variable.
> >>>
> >>>I have also fixed a conflict with guenther@'s recent fetch.c commit.
> >>>
> >>>Would anyone like to ok this latest version?
> >>
> >>I was reviewing this and I couldn't help finding it unnecessarily
> >>cumbersome.
> >>
> >>I propose this diff (ontop on the already proposed and committed diff).
> >>Apart from making the code simpler, this diff will change two things:
> >
> >Thanks for simplifying this. The original diff used an environment
> >variable and for consistency with the existing code that deals with
> >environment variables, I implemented it within auto_fetch().
> >
> >When I changed it to use a command-line option, I continued implementing
> >it within auto_fetch() because that was where my original code was. But
> >as your diff shows, that's unnecessary, so I appreciate your work in
> >making it less cumbersome.
> >
> >I agree with your diff except for this part:
> >
> >>1. You may specify -U as many times as you please, using only the last
> >> one. This is the behavious I'd expect.
> >
> >What is the use case for specifying multiple -U instances and only
> >choosing the last one? To me that sounds like something I would
> >accidentally do as opposed to something I would intentionally do, so
> >that's why my code tried to prevent it.
>
> Mainly because that's how I would expect any option to work. -o, just to
> give one example.
>
> hmmm.. use case:
>
> getfile() {
> ftp -U 'firefox' "$@"
> }
>
> getfile http://foo.bar/baz1
> getfile http://foo.bar/baz2
>
> getfile -U 'chrome' http://foo.bar/baz3
Ah, thanks. I most likely won't use it that way but I see the point.
OK lteo@
