Default port to 11211

[jolks]

Update for Default port to 11211 in URLs #207

[ronkorving]

I think the (.*) adds nothing to the expression (since you're not pegging the expression to the starting point of the string). Also not sure what the point of the {1,} is here?

[jolks]

The (.*) is to match any IP address that is defined when connecting. The {1,} is for (\d+), in other words (\d+){1,}$ is to detect any presence of port number. If no port number is detected regardless of any IP address, the 11211 will be appended as default port. Actually i just used the same regular expression as originally defined in line 140.

[ronkorving]

Ah, thanks. It's good to know where that regex originated.
You don't see anything wrong with it though?

(.*) does not match IP addresses (or really anything for that matter), right?
And {,1} seems redundant to me, am I wrong?

[jolks]

(.*) is required to detect not only IP address e.g. 127.0.0.1 but also domain name e.g. mymemcached.example.com. You are right, {1,} is redundant because the + is actually equivalent to {1,}. Let me update the code.

[ronkorving]

.* literally means "any type of character, repeated zero or more times". It adds nothing to the test.

[jolks]

Then let's use (.+) expression. I just pushed up an update.

[ronkorving]

Could you add a unit test to prove this works? I think there's already a test file around connections that you could extend.

[jolks]

Sure, i just pushed up the unit test for it.

[3rd-Eden]

I think that an ipv6 address will break horribly by this

[jolks]

Correct me if i'm wrong but i think node-memcached currently doesn't support ipv6 address...yet. i don't see any related tests or examples. Are we going to provide ipv6 support in the upcoming new versions? If yes, how can i update this current change to fit into the big picture?

[jolks]

i did a quick test on node REPL:-

> var server = '[1fff:0:a88:85a3::ac1f]:8001';
undefined
> server
'[1fff:0:a88:85a3::ac1f]:8001'
> server.match(/(.+):(\d+)$/)
[ '[1fff:0:a88:85a3::ac1f]:8001',
  '[1fff:0:a88:85a3::ac1f]',
  '8001',
  index: 0,
  input: '[1fff:0:a88:85a3::ac1f]:8001' ]
> server.match(/(.+):(\d+)$/);
[ '[1fff:0:a88:85a3::ac1f]:8001',
  '[1fff:0:a88:85a3::ac1f]',
  '8001',
  index: 0,
  input: '[1fff:0:a88:85a3::ac1f]:8001' ]
> var server = '[1fff:0:a88:85a3::ac1f]';
undefined
> server.match(/(.+):(\d+)$/);
null

My part can still handle ipv6 address unless maybe i have to do more thorough tests.

[ronkorving]

Why on earth would it break ip6? Before the colon it'll match any character(s).

In case of doubt, just augment the unit test to have a wide variation of host types (domain name, ipv4, ipv6), and no matter how people change this regex, your test will find breakage.

[3rd-Eden]

@ronkorving I must have been looking at old cold as I thought it assume that numbers that are followed after a : was assumed a port. But it looks fine by me so i have no problem merging it in.

[ronkorving]

Here we go! :)

[ronkorving]

Thanks @jolks

[jolks]

Cheers @ronkorving and @3rd-Eden !

[kmanoraj]

This code is breaking when I passed multiple hosts as an array

[ronkorving]

@kmanoraj The (private) connect() function only accepts strings, so if there is a problem, this PR should not have caused it I think.