ap / rename

Rename multiple files
http://plasmasturm.org/code/rename/
Other
125 stars 11 forks source link

Added flag for 'slugify' and 'rslugify #3

Closed ghost closed 7 years ago

ghost commented 7 years ago

Hi, I've just added another flag, similar to the functionality of 'nows', and 'rews' that replaces spaces with dashes, and vice versa.

ap commented 7 years ago

I like the requirement, it’s a good point. But I don’t like the solution… slugify is a weird name to give this, and rslugify is on another league of terrible entirely (if I may be so frank).

How about an optional argument on --nows/--rews which defaults to _? (Meaning that instead of “--slugify” you would use “--nows=-”.)

ghost commented 7 years ago

Ok, I'll take a look at that. Another option would be to create a slugcase flag instead. It could act like the camelcase flag, converting any string to slug case. What do you think?

ap commented 7 years ago

That’s the problem with the name… I have no idea what you mean to describe by slug case. Are you talking about the same thing you were talking before? Presumably not, due to the reference to camelcase. But the reference doesn’t help me picture anything.

ghost commented 7 years ago

Oh sorry, 'slug' case is a popular term in web dev, at least in the US. Example: A string formatted-like-this would be called a slug, and is a popular format for urls. My goal with a slugcase flag would be to parse and convert any string into a slug format (maybe a better name).

I'll take a stab this weekend and you can check on the next commit and see what you think.

ghost commented 7 years ago

I've attempted to do as you suggested and just have nows accept an optional argument like so: rename --nows=- *.

I've got that working, however when not using an option, the command fails on the first file in the argument list. Example: rename --nows some\ file.txt some\ text.txt will only rename some text.txt to some_text.txt but leaves some file.txt unchanged. If only one file is provided, the command never executes, just breaks.

I've been diving in the docs but can't seem to nail down the issue. Below is what I've added. Sorry, it's my first time using Perl.


sub nows_handler {
    my ($opt_name, $opt_value) = @_;
    $opt_value eq '-' ? push @EXPR, "s/[-[:blank:]]+/-/g" : push @EXPR, "s/[_[:blank:]]+/_/g";
}

sub rews_handler {
    my ($opt_name, $opt_value) = @_;
    $opt_value eq '-' ? push @EXPR, "y/-/ /" : push @EXPR, "y/_/ /";
}

my %library = (
        ...
    # nows      => 's/[_[:blank:]]+/_/g',
    # rews      => 'y/_/ /',
        ...
);

GetOptions(
    ...
    'nows:s' => \&nows_handler,
        ...
    'rews:s'  => \&rews_handler,
        ...
    map { my $recipe = $_; $recipe => sub { push @EXPR, $library{ $recipe } } } keys %library,
) or pod2usage();
ap commented 7 years ago

Nothing to be sorry for in your first try at Perl!

I've got that working, however when not using an option, the command fails on the first file in the argument list.

Yeah, good point, I didn’t anticipate that issue. Well, it doesn’t fail so much as it takes the filename as the value for the --nows switch (which I assume you missed because your code doesn’t use the value for anything else than a comparison with -). That’s an issue with my proposed solution.

I thought about it and decided the best approach is probably to require the argument to be attached to the switch with a =, i.e. --nows=- will treat the dash as its value but --nows - won’t.

That’s not something you can do with just Getopt::Long’s own API, though, because it doesn’t provide any way to distinguish --foo=bar from --foo bar. That’s mostly a good thing, because these should not be different ordinarily, but it gets in the way in this case.

I puzzled about the best way to hack around it and in the end I found a stupidly simple way by just preprocessing @ARGV to tack the =_ default onto any bare --nows/--rews switches, so that Getopt::Long never gets to see the bare form, which sidesteps the problem.

That’s pushed in branch nowsparam for now until I write some docs for it.

ghost commented 7 years ago

Yea I was wondering if it was trying to apply the value of the filename to --nows. Thanks for fixing it! I think Larry Wall must be a mad man or a genius. I can update the man pages on the nowsparam branch.