Skip to content

Conversation

eksperimental
Copy link
Contributor

Closes #717

It rewrites "http" to "https" when building source_url_pattern.

lib/ex_doc.ex Outdated
:error
end)
do
( %URI{parsed_url | scheme: "https", port: 443}

Choose a reason for hiding this comment

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

There is no whitespace around parentheses/brackets most of the time, but here there is.

lib/ex_doc.ex Outdated

defp guess_url(url = <<"https://bitbucket.org/", _ :: binary>>, ref) do
append_slash(url) <> "src/#{ref}/%{path}#cl-%{line}"
defp guess_url(url, ref) when is_binary(url) do
Copy link
Member

Choose a reason for hiding this comment

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

What if we rely more on pattern matching and functions? Something like:

def guess_url(url, ref) do
  with {:ok, host_with_path} <- http_or_https(url),
       {:ok, pattern} <- known_pattern(host_with_path, ref) do
    append_slash(url) <> pattern
  else
    _ -> url
  end
end

defp http_or_https("http://" <> rest), do: {:ok, rest}
defp http_or_https("https://" <> rest), do: {:ok, rest}
defp http_or_https(_), do: :error

defp known_pattern("github.com/" <> _, ref),
  do: {:ok, "blob/#{ref}/%{path}#L%{line}"}
defp known_pattern("gitlab.com/" <> _, ref),
  do: {:ok, "blob/#{ref}/%{path}#L%{line}"}
defp known_pattern("bitbucket.org/" <> _, ref),
  do: {:ok, "src/#{ref}/%{path}#cl-%{line}"}
defp known_pattern(_),
  do: :error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

definitely much better.

Closes elixir-lang#717

It rewrites "http" to "https" when building source_url_pattern.
@eksperimental
Copy link
Contributor Author

ok. this is ready to merge

@josevalim josevalim merged commit a22d6b9 into elixir-lang:master May 21, 2017
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@eksperimental eksperimental deleted the guess_url branch May 21, 2017 15:39
@eksperimental
Copy link
Contributor Author

Thank you @josevalim

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

Successfully merging this pull request may close these issues.

2 participants