mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

Feature Request: Mojo::File method to get/set file extension #1448

Closed srchulo closed 3 years ago

srchulo commented 4 years ago

I think it would be useful to have an ext method on Mojo::File that allows you to:

# returns 'txt'
my $ext = path('file.txt')->ext;
# returns Mojo::File with path 'file.csv'
path('file.txt')->ext('csv')

# returns Mojo::File with path 'file'
path('file.txt')->ext('')

# providing a leading . works too
path('file.txt')->ext('.csv')

Below is a quick implementation that I believe does what was described above:

sub ext {
  my $self = shift;
  return ($self =~ /\.([^.]*)/)[0] // '' unless @_;

  my $ext = shift // '';
  if ($ext ne '' and index($ext, '.') != 0 ) {
    $ext = ".$ext";
  }

  my $new_path = $$self;
  unless ($new_path =~ s/\.[^.]*/$ext/) {
    $new_path .= $ext;
  }

  return $self->new($new_path);
}

I'm curious if others think this would be useful, or if there are any issues/complications I'm not thinking of?

Grinnz commented 4 years ago

The difference in behavior between get and set while not being a real attribute leads me to think it's more confusing than it's worth.

Grinnz commented 4 years ago

Also, it needs to consider files with multiple extensions

s1037989 commented 4 years ago

This would be extremely useful! I often find myself wishing that Mojo::File had that built in. I always end up creating a quick Role package at the top of my module to get it done, but it certainly gets repetitive. Also, I never seem to be consistent about it from project to project. I haven't released a CPAN module for such a role because I haven't stopped to think about what a good implementation should look like.

Having it built in would be so, so great!!

On Tue, Dec 10, 2019, 12:21 PM Adam Hopkins notifications@github.com wrote:

I think it would be useful to have an ext method on Mojo::File https://metacpan.org/pod/Mojo::File that allows you to:

  • Get an extension when you provide no arguments

returns 'txt'

my $ext = path('file.txt')->ext;

  • Set an extension and return a new Mojo::File

returns Mojo::File with path 'file.csv'

path('file.txt')->ext('csv')

returns Mojo::File with path 'file'

path('file.txt')->ext('')

providing a leading . works too

path('file.txt')->ext('.csv')

Below is a quick implementation that I believe does what was described above:

sub ext { my $self = shift; return ($self =~ /.([^.]*)/)[0] // '' unless @_;

my $ext = shift // ''; if ($ext ne '' and index($ext, '.') != 0 ) { $ext = ".$ext"; }

my $new_path = $$self; unless ($new_path =~ s/.[^.]*/$ext/) { $new_path .= $ext; }

return $self->new($new_path); }

I'm curious if others think this would be useful, or if there are any issues/complications I'm not thinking of?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mojolicious/mojo/issues/1448?email_source=notifications&email_token=AAD6K6R6ML2FY7W2BJTXH2TQX7M3BA5CNFSM4JZC2ZLKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4H7RHWUQ, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD6K6TUGVHEYMBCDXSHEKLQX7M3BANCNFSM4JZC2ZLA .

srchulo commented 4 years ago

@Grinnz although it isn't a real attribute on the Mojo::File instance, an extension is a real attribute on files, so from the user's perspective I feel like it isn't any different from a real attribute. And really, the extension could be parsed as an attribute when creating a new object and then used appropriately everywhere else, but I think that may be more complicated.

The multiple extensions is a good point. I think that can be solved easily by just looking at the last part using to_array:

return (@{$self}[-1] =~ /\.(.*)/)[0] // '' unless @_;

And similar for setting the last part to replace the extension.

srchulo commented 4 years ago

Maybe something like:

sub ext {
  my $self = shift;

  my $parts = $self->to_array;
  return ($parts->[-1] =~ /\.(.*)/)[0] // '' unless @_;

  my $ext = shift // '';
  if ($ext ne '' and index($ext, '.') != 0 ) {
    $ext = ".$ext";
  }

  unless ($parts->[-1] =~ s/\..*/$ext/) {
    $parts->[-1] .= $ext;
  }

  return $self->new(@$parts);
}

Although that doesn't account for the case where it's a directory. Maybe it could just return at the top if it's a directory.

Grinnz commented 4 years ago

Directories have extensions too. (most commonly .d)

srchulo commented 4 years ago

Ah, right. Scratch that part then :)

s1037989 commented 4 years ago

Could extensions be handled like files, where extensions have parents, children, and siblings? The extension would always operate on the basename of the Mojo::File instance.

On Tue, Dec 10, 2019, 1:48 PM Adam Hopkins notifications@github.com wrote:

Ah, right. Scratch that part then :)

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mojolicious/mojo/issues/1448?email_source=notifications&email_token=AAD6K6RX4YDRY6CY5Y7UQBTQX7XBXA5CNFSM4JZC2ZLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGQT2MQ#issuecomment-564215090, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD6K6XFAGMERKUCCUEELF3QX7XBXANCNFSM4JZC2ZLA .

kiwiroy commented 4 years ago

Likewise, I often need this functionality and hack around/copy-pasta solutions.

Any comments on this role?

kraih commented 4 years ago

If this is really such a popular feature i don't mind putting an implementation up for a vote. But so far i've not seen a proposal here where i liked the code.

srchulo commented 4 years ago

@kiwiroy I like the idea of the moniker method. I'm not a big fan of having two methods to get the and switch the extension, but maybe that is less confusing than having one. To me it feels more natural to have one method that does both, and more like what a user may expect from a file object.

srchulo commented 4 years ago

@kiwiroy I do like your implementation. Much cleaner than mine :)

kiwiroy commented 4 years ago

The gist above has been moved to CPAN.

kraih commented 4 years ago

I'm not opposed to the feature, but implementations so far seem rather complex and interest fairly limited.

kraih commented 4 years ago

An extname method has been added to Mojo::File. https://github.com/mojolicious/mojo/commit/44659559d6cd287fd25415acee0710c9e39380ab

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It may be closed if no further activity occurs. Thank you for your contributions.

kraih commented 3 years ago

No new feedback, do we consider this resolved?

srchulo commented 3 years ago

I think so. With extname added by you and Mojo::File::Role::Extension by @kiwiroy , I think all of my use cases are covered.

srchulo commented 3 years ago

Thanks for those additions!