Skip to content
This repository has been archived by the owner on Mar 10, 2020. It is now read-only.

Exclude port from path if config.port not set. #162

Merged
merged 2 commits into from
Dec 19, 2015

Conversation

alexmingoia
Copy link
Contributor

Fixes #161.

@dignifiedquire
Copy link
Contributor

thanks. could you add a test for this please?

@alexmingoia
Copy link
Contributor Author

Of course. Tests added.

const opts = {
method: files ? 'POST' : 'GET',
uri: `${config.protocol}://${config.host}:${config.port}${config['api-path']}${path}?${Qs.stringify(qs, {arrayFormat: 'repeat'})}`,
uri: `${config.protocol}://${config.host}${port}${config['api-path']}${path}?${Qs.stringify(qs, {arrayFormat: 'repeat'})}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handling this with the url module is probably best. It cleans up that template a lot. Not a huge issue though.

let uri = url.format({
  protocol: config.protocol,
  hostname: config.host,
  port: config.port,
  pathname: `${config['api-path']}/${path}`.replace(/\/+/g, '/'), // Double slashes don't matter, but it looks nicer.
  query: qs
})

It will handle the undefined port and not include it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

daviddias added a commit that referenced this pull request Dec 19, 2015
Exclude port from path if `config.port` not set.
@daviddias daviddias merged commit 285a7e3 into ipfs-inactive:master Dec 19, 2015
@harlantwood
Copy link
Contributor

Nice, thanks. cc @jacksenechal.

@daviddias
Copy link
Contributor

Thank you for fixing this and adding the test :):)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants