Home of the Squeezebox™ & Transporter® network music players.
Results 1 to 9 of 9
  1. #1
    Perl Commando Dan Sully's Avatar
    Join Date
    Apr 2005
    Location
    Daly City, CA
    Posts
    2,864

    fix for bug 1312

    This should do it - getting rid of the URL scheme crap. This is a lot easier
    (and makes code go away), with additional error checking.

    Please review - it'd be nice to get it into 6.0.1

    -D
    --
    Ya gotta love UNIX, where else do you wonder whether
    you can kill a zombie spawned by a daemon's fork?

  2. #2
    Robert Moser
    Guest

    Re: fix for bug 1312

    Dan Sully wrote:
    > This should do it - getting rid of the URL scheme crap. This is a lot
    > easier
    > (and makes code go away), with additional error checking.
    >
    > Please review - it'd be nice to get it into 6.0.1
    >
    > -D


    Is there a way through object methods to determine whether or not a URL
    is remote? If so, the 'download' parameter could go away completely, in
    favor of building the URL in TT:
    <a href="[% IF itemobj.isRemoteURL %][% itemobj.url %][% ELSE %][%
    webroot %]music/[% itemobj.id %]/download[% END %]">blah blah</a>

    Or for that matter, just make a downloadURL method on the track object
    so we can do this:
    <a href="[% itemobj.downloadURL %]">blah blah</a>

  3. #3
    Perl Commando Dan Sully's Avatar
    Join Date
    Apr 2005
    Location
    Daly City, CA
    Posts
    2,864

    Re: fix for bug 1312

    * Robert Moser shaped the electrons to say...

    >Is there a way through object methods to determine whether or not a URL
    >is remote? If so, the 'download' parameter could go away completely, in
    >favor of building the URL in TT:
    ><a href="[% IF itemobj.isRemoteURL %][% itemobj.url %][% ELSE %][%
    >webroot %]music/[% itemobj.id %]/download[% END %]">blah blah</a>
    >
    >Or for that matter, just make a downloadURL method on the track object
    >so we can do this:
    ><a href="[% itemobj.downloadURL %]">blah blah</a>


    Yes - these should really be a method on the object - but that's a bigger
    change than I can deal with right now.

    -D
    --
    "It has become appallingly obvious that our technology has exceeded our humanity." - Albert Einstein

  4. #4
    NOT a Slim Devices Employee kdf's Avatar
    Join Date
    Apr 2005
    Posts
    9,493

    Re: Re: fix for bug 1312

    Quoting Dan Sully <dan (AT) slimdevices (DOT) com>:

    > * Robert Moser shaped the electrons to say...
    >
    > >Is there a way through object methods to determine whether or not a URL
    > >is remote? If so, the 'download' parameter could go away completely, in
    > >favor of building the URL in TT:
    > ><a href="[% IF itemobj.isRemoteURL %][% itemobj.url %][% ELSE %][%
    > >webroot %]music/[% itemobj.id %]/download[% END %]">blah blah</a>
    > >
    > >Or for that matter, just make a downloadURL method on the track object
    > >so we can do this:
    > ><a href="[% itemobj.downloadURL %]">blah blah</a>

    >
    > Yes - these should really be a method on the object - but that's a bigger
    > change than I can deal with right now.


    how about, go with the quick fix, then retarget (instead of marking as fixed)
    the bug for 6.1 with a comment that it should be a method

  5. #5
    Perl Commando Dan Sully's Avatar
    Join Date
    Apr 2005
    Location
    Daly City, CA
    Posts
    2,864

    Re: Re: fix for bug 1312

    * kdf shaped the electrons to say...

    >> Yes - these should really be a method on the object - but that's a bigger
    >> change than I can deal with right now.

    >
    >how about, go with the quick fix, then retarget (instead of marking as fixed)
    >the bug for 6.1 with a comment that it should be a method


    That was my plan. How does the patch look to everyone?

    Death to URLs!

    -D
    --
    On second thought, let's not go to Camelot. It is a silly place.

  6. #6
    NOT a Slim Devices Employee kdf's Avatar
    Join Date
    Apr 2005
    Posts
    9,493

    Re: Re: Re: fix for bug 1312

    Quoting Dan Sully <dan (AT) slimdevices (DOT) com>:

    > * kdf shaped the electrons to say...
    >
    > >> Yes - these should really be a method on the object - but that's a bigger
    > >> change than I can deal with right now.

    > >
    > >how about, go with the quick fix, then retarget (instead of marking as

    > fixed)
    > >the bug for 6.1 with a comment that it should be a method

    >
    > That was my plan. How does the patch look to everyone?
    >
    > Death to URLs!


    it seems to work ok for me. I dont have any mapped drives holding my music, but
    I do have moodlogic and itunes going.

    -kdf

  7. #7
    Perl Commando Dan Sully's Avatar
    Join Date
    Apr 2005
    Location
    Daly City, CA
    Posts
    2,864

    Re: Re: Re: fix for bug 1312

    * kdf shaped the electrons to say...

    >it seems to work ok for me. I dont have any mapped drives holding my music, but
    >I do have moodlogic and itunes going.


    Ok - good. I have a small change to de-url encode the resulting filename on the download.

    Going to check it in shortly.

    -D
    --
    It's the wrong trousers Gromit, and they've gone wrong!

  8. #8
    Robert Moser
    Guest

    Re: Re: Re: fix for bug 1312

    Dan Sully wrote:
    > * kdf shaped the electrons to say...
    >
    >>> Yes - these should really be a method on the object - but that's
    >>> a bigger change than I can deal with right now.

    >>
    >>
    >> how about, go with the quick fix, then retarget (instead of marking
    >> as fixed) the bug for 6.1 with a comment that it should be a method
    >>

    >
    >
    > That was my plan. How does the patch look to everyone?
    >
    > Death to URLs!
    >
    > -D

    Slight problem, for the download URL for the non-remoteURL case, we need
    to use the correct webroot, just in case someone is using the
    /slimserver/ proxy method.

    So this:

    $params->{'download'} = sprintf('/music/%d/download', $track->id());

    becomes this:

    $params->{'download'} = sprintf('%smusic/%d/download',
    $params->{'webroot'}, $track->id());

  9. #9
    Perl Commando Dan Sully's Avatar
    Join Date
    Apr 2005
    Location
    Daly City, CA
    Posts
    2,864

    Re: Re: Re: fix for bug 1312

    * Robert Moser shaped the electrons to say...

    >Slight problem, for the download URL for the non-remoteURL case, we need
    >to use the correct webroot, just in case someone is using the
    >/slimserver/ proxy method.


    Thanks - applied.

    -D
    --
    It is dark. You are likely to be eaten by a grue.

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •