Bug #71566 for Catalyst-Runtime: have copy_to return filename of resultant file

Mon Oct 10 06:53:02 2011 dean [...] fragfest.com.au - Ticket created

Subject:

have copy_to return filename of resultant file

hello currently if a software artist uses the copy_to($path) function, where $path is directory - then the file is copied to "$path/$tmpfilename". if $path is actually a full filename, then this is used. in the case where saving using the $tmpfilename is convenient (as it generates a unique filename for the artist), it would be even more convenient if the copy_to function returned the resultant filename. this would not break backward compatibility (unless another artist is explicitly looking for copy_to() == '1') so the interface would be enhanced to this sort of functionality if (my $filename = $obj->copy_to("/some/place")) { $c->stash->{message} = "File was saved as $filename"; } else { # other }

Mon Oct 10 07:18:46 2011 bobtfish [...] bobtfish.net - Correspondence added

On Mon Oct 10 06:53:02 2011, djzort wrote: Show quoted text

> hello > > currently if a software artist uses the copy_to($path) function

which copy_to function? Catalyst does not supply a function of this name. Show quoted text

> , where > $path is directory - then the file is copied to "$path/$tmpfilename". if > $path is actually a full filename, then this is used. > > in the case where saving using the $tmpfilename is convenient (as it > generates a unique filename for the artist), it would be even more > convenient if the copy_to function returned the resultant filename. this > would not break backward compatibility (unless another artist is > explicitly looking for copy_to() == '1') > > so the interface would be enhanced to this sort of functionality > > if (my $filename = $obj->copy_to("/some/place")) {

Oh, right, I assume you mean the method (not function) on Catalyst::Request::Upload. I don't see the utility to Catalyst generating a temp filename for you (when you have to pass it the directory name anyway!). Why not just file File::Temp? Also, different behavior based on context breaks the principle of least surprise! If I try to copy a file to something which already exists, I would expect an exception, not the behavior to differ depending upon what type of thing the directory that already exists is.

Mon Oct 10 07:18:48 2011 The RT System itself - Status changed from 'new' to 'open'

Mon Oct 10 07:18:49 2011 bobtfish [...] bobtfish.net - Status changed from 'open' to 'rejected'

Mon Oct 10 17:36:47 2011 dean [...] fragfest.com.au - Correspondence added

Sorry, i clicked through from cpan.org and didnt realise the bug went to the parent package. Anyway. The current behaviour may be considered suprising. For example. where $obj->filename eq 'IMG_1234.jpg' and $obj->tempname eq '/tmp/advewrv.jpg' $obj->copy_to("/some/path/"); results in "/some/path/advewrv.jpg" rather than "/some/path/IMG_1234.jpg" this makes sense from the point of view that "cp advewrv.jpg /some/path/" but is, to me, very counter intuitive and hence 'suprising'. its certainly not documented. my original request should perhaps be that copy_to always uses the $obj->filename. additionally, it would useful to me if the copy_to method could be told whether it is allowed to override an existing file. anyway thanks for your good work, i appreciate your consideration of my thoughts.

Mon Oct 10 17:36:50 2011 The RT System itself - Status changed from 'rejected' to 'open'

Tue Apr 28 14:57:44 2015 jjnapiork [...] cpan.org - Correspondence added

On Mon Oct 10 17:36:47 2011, djzort wrote: Show quoted text

> Sorry, i clicked through from cpan.org and didnt realise the bug went to > the parent package. Anyway. > > The current behaviour may be considered suprising. > > For example. > > where $obj->filename eq 'IMG_1234.jpg' and $obj->tempname eq > '/tmp/advewrv.jpg' > > $obj->copy_to("/some/path/"); > > results in "/some/path/advewrv.jpg" rather than "/some/path/IMG_1234.jpg" > > this makes sense from the point of view that "cp advewrv.jpg > /some/path/" but is, to me, very counter intuitive and hence > 'suprising'. its certainly not documented. > > my original request should perhaps be that copy_to always uses the > $obj->filename. > > additionally, it would useful to me if the copy_to method could be told > whether it is allowed to override an existing file. > > anyway thanks for your good work, i appreciate your consideration of my > thoughts.

I added docs to the upload object as you suggested to make what is happening more clear. This will ship with the next stable catalyst.

Tue Apr 28 14:57:47 2015 jjnapiork [...] cpan.org - Status changed from 'open' to 'patched'

Thu Oct 29 16:48:56 2015 jjnapiork [...] cpan.org - Correspondence added

I'm going to close this ticket since I think the docs are better now. Please reopen if you think otherwise -jnap On Tue Apr 28 14:57:44 2015, JJNAPIORK wrote: Show quoted text

> On Mon Oct 10 17:36:47 2011, djzort wrote:

> > Sorry, i clicked through from cpan.org and didnt realise the bug went > > to > > the parent package. Anyway. > > > > The current behaviour may be considered suprising. > > > > For example. > > > > where $obj->filename eq 'IMG_1234.jpg' and $obj->tempname eq > > '/tmp/advewrv.jpg' > > > > $obj->copy_to("/some/path/"); > > > > results in "/some/path/advewrv.jpg" rather than > > "/some/path/IMG_1234.jpg" > > > > this makes sense from the point of view that "cp advewrv.jpg > > /some/path/" but is, to me, very counter intuitive and hence > > 'suprising'. its certainly not documented. > > > > my original request should perhaps be that copy_to always uses the > > $obj->filename. > > > > additionally, it would useful to me if the copy_to method could be > > told > > whether it is allowed to override an existing file. > > > > anyway thanks for your good work, i appreciate your consideration of > > my > > thoughts.

> > I added docs to the upload object as you suggested to make what is > happening more clear. This will ship with the next stable catalyst.

Thu Oct 29 16:49:05 2015 jjnapiork [...] cpan.org - Status changed from 'patched' to 'resolved'