New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port return type from url.parse() #833

Closed
nnposter opened this Issue Apr 6, 2017 · 0 comments

Comments

Projects
None yet
1 participant
@nnposter

nnposter commented Apr 6, 2017

Function url.parse() separates a given URL into individual components, such as the scheme, host, port, and path.

As of now the port part is being returned as string. I would like to propose that port return type is changed to number, technically an integer.

This will make the function more consistent with other libraries, which frequently expect a port parameter in the form of the full-fledged table or a number, but not a string.

The switch will also reduce obscure ambiguities that already exist in the code base. As an example, several scripts contain a code like this:

local function getHostPort(parsed)
  local host, port = parsed.host, parsed.port
  -- if no port was found, try to deduce it from the scheme
  if ( not(port) ) then
    port = (parsed.scheme == 'https') and 443
    port = port or ((parsed.scheme == 'http') and 80)
  end
  return host, port
end

Such a function will sometimes return the port as a number and other times as a string.

A corresponding patch already exists here. It covers both the change itself and a corresponding clean-up of the consuming code.

The change looks pretty non-controversial but I am cognizant of the fact that it changes behavior of a standard library.

Please let me know if you have any questions or concerns. Otherwise I will commit the change in a few weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment