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

fixes #268 Addressable::URI.heuristic_parse: uri starts with a digit … #296

Merged
merged 1 commit into from May 31, 2018

Conversation

Mifrill
Copy link
Contributor

@Mifrill Mifrill commented Apr 16, 2018

…and has specified port

@@ -6252,6 +6252,18 @@ def to_str
end
end

describe Addressable::URI, "when given the input " +
"'7777.example.org:8089'" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Layout/MultilineOperationIndentation: Align the operands of an expression spanning multiple lines.

@@ -6252,6 +6252,18 @@ def to_str
end
end

describe Addressable::URI, "when given the input " +
Copy link
Collaborator

Choose a reason for hiding this comment

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

Style/LineEndConcatenation: Use \ instead of + or << to concatenate those strings.

@@ -6252,6 +6252,17 @@ def to_str
end
end

describe Addressable::URI, "when given the input which start with digits and has specified port" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

Metrics/LineLength: Line is too long. [99/80]

@@ -5498,7 +5498,7 @@ def to_s
expect(uri.tld).to eq("uk")
end

context "witch " do
context "which " do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

preamble: #294 (review)

@Mifrill
Copy link
Contributor Author

Mifrill commented Apr 16, 2018

Closes #275

@@ -18,4 +18,5 @@

RSpec.configure do |config|
config.warnings = true
config.filter_run_when_matching :focus
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI this require rspec core >= 3.5

https://relishapp.com/rspec/rspec-core/v/3-5/docs/filtering/filter-run-when-matching (switch version to 3.4 and docs can't be found)

the Gemfile here says rspec 3.0:

gem 'rspec', '~> 3.0'

I think that should be bumped if this goes in

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dentarg you know better what is better for this repo
Tell me to change the version of the gem or delete this line?

I will do as you say, thank you

Copy link
Collaborator

@dentarg dentarg Apr 16, 2018

Choose a reason for hiding this comment

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

might be better to remove this line and submit it as a suggestion in another pull request, with the bump in Gemfile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, i'll do it later, thank you

@@ -194,6 +194,8 @@ def self.heuristic_parse(uri, hints={})
uri.sub!(/^file:\/+/i, "file:///")
when /^\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3}/
uri.sub!(/^/, hints[:scheme] + "://")
else
uri = hints[:scheme] + "://" + uri if uri[/\A\d+\..*:\d+\z/]
Copy link
Owner

Choose a reason for hiding this comment

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

Why not just make this another when clause?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, it's much better, thank you

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 this pull request may close these issues.

None yet

4 participants