Skip to content
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

extra::url::to_str doesn't include port #9451

Closed
chris-morgan opened this issue Sep 24, 2013 · 0 comments · Fixed by #9927
Closed

extra::url::to_str doesn't include port #9451

chris-morgan opened this issue Sep 24, 2013 · 0 comments · Fixed by #9927

Comments

@chris-morgan
Copy link
Member

extra::url::to_str doesn't include the port, leading to an incorrect string representation of the URL.

If the port is not None, it should be tacked on after the host name with a colon before it. I don't think intelligence in omitting it for the default port of a scheme (e.g. if it's Some(~"80") with the scheme ~"http") is desirable.

(... hang on: why is port a ~str rather than a u16? Out of scope for this issue.)

See also chris-morgan/rust-http#16.

bors added a commit that referenced this issue Oct 18, 2013
@bors bors closed this as completed in 1093730 Oct 18, 2013
flip1995 pushed a commit to flip1995/rust that referenced this issue Oct 20, 2022
Add `manual_filter` lint for `Option`

Share much of its implementation with `manual_map` and should greatly benefit from its previous feedback.
I'm sure it's possible to even more refactor both and would gladly take input on that as well as any clippy idiomatic usage, since this is my first lint addition.

I've added the lint to the complexity section for now, I don't know if every new lint needs to go in nursery first.

The matching could be expanded to more than `Some(<value>)` to lint on arbitrary struct matching inside the `Some` but I've left it like it was for `manual_map` for now. `needless_match::pat_same_as_expr` provides a more generic match example.

close rust-lang/rust-clippy#8822

changelog: Add lint [`manual_filter`] for `Option`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant