pid / speakingurl

Generate a slug – transliteration with a lot of options
http://pid.github.io/speakingurl/
BSD 3-Clause "New" or "Revised" License
1.12k stars 84 forks source link

Convert number to string #75

Closed niftylettuce closed 8 years ago

niftylettuce commented 8 years ago

can you check if (typeof input === 'number') input = input.toString();?

e.g. getSlug(44) should output '44' but it outputs ''

leocaseiro commented 8 years ago

Temporary solution:

getSlug('44');

or perhaps:

getSlug('' + 44);
niftylettuce commented 8 years ago

Yeah that works but I want to pass a number argument On May 29, 2016 8:10 PM, "Leo Caseiro" notifications@github.com wrote:

Can you try:

getSlug('44');

or perhaps:

getSlug('' + 44);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pid/speakingurl/issues/75#issuecomment-222390889, or mute the thread https://github.com/notifications/unsubscribe/AAf7hYKYopybz-enof8dtBdDV71CofR6ks5qGirXgaJpZM4IpWQ5 .

simi commented 8 years ago

Just ensure it is number (and type to string) before passing to getSlug.

As you see in API docs getSlug is expecting string as input.

leocaseiro commented 8 years ago

Yeah, that's a way you to do it:

var myint = 44;
getSlug('' + myint);

Ref: http://stackoverflow.com/a/5765401/3415716

niftylettuce commented 8 years ago

No, that's not the way I was suggesting. I want to pass as an argument the number. In other words, I do not want to have to call .toString() or the shorter + '' version. I'd like this to accept numbers. It's a really odd edge case, I know, but this shouldn't be hard to add, maybe an extra 2 lines of code. Do you need me to submit a PR?

On Sun, May 29, 2016 at 10:09 PM, Leo Caseiro notifications@github.com wrote:

Yeah, that's the way you do it:

var myint = 44;getSlug('' + myint);

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/pid/speakingurl/issues/75#issuecomment-222398567, or mute the thread https://github.com/notifications/unsubscribe/AAf7hbnrxEEK28F6qe_1RYga027VQLsIks5qGkbDgaJpZM4IpWQ5 .

@niftylettuce

simi commented 8 years ago

Yeah, this is 2 extra lines of code. Then someone will came here and ask to add another two lines for another data type. In the end, there will be big condition handling all those types.

What I suggest is to keep it in this way as it is now and move that responsibility outside of this library. In API docs is written that getSlug expects string. You need to handle that in your code.

If you need to pass number in there, you can introduce your own wrapper on top of the getSlug for example or call toString() before passing to getSlug.

I'm :-1: on this feature since it will solve your problem, but it will add some complexity not used in 99% usage.

leocaseiro commented 8 years ago

I agree with @simi.

First, I don't see any use case when an integer needs to convert into URL(ish)

If you need to replace your number in slug, create a pre-function for your full library than:

var getIntegerSlug = function(input) {
    if (typeof input === 'number') {
        input = input.toString(); 
    }

    return getSlug(integerValue);
}

USAGE:

   getIntegerSlug(integerValue);
niftylettuce commented 8 years ago

Yeah those things are obvious, this was a simple request to add two lines of code, I'll just close.

pid commented 8 years ago

But, this ticket opens another question, should getSlug return/throw an error if input is not a string or return false? To return an empty string feels not the best.... what you are thinking about? Should we change this?