#164 Transform common project URLs to repository #169
Conversation
| @@ -27,7 +27,11 @@ | |||
| from pygount.common import mapped_repr | |||
| from pygount.git_storage import GitStorage, git_remote_url_and_revision_if_any | |||
|
|
|||
| GIT_REPO_REGEX = re.compile(r"^(https?://|git@)") | |||
| HAS_URL_PREFIX = re.compile(r"^(https?://)") | |||
There was a problem hiding this comment.
Naming: It's not a boolean rather a regex, so: URL_REGEX.
We might also want to be a bit more precise because a log of things can be an URL, so possibly even better: HTTP_URL_REGEX
| # TODO#113: Find a way to exclude the ugly temp folder from the source path. | ||
| result.extend(self._paths_and_group_to_analyze(git_storage.temp_folder)) | ||
| else: | ||
| has_url_prefix = re.match(HAS_URL_PREFIX, source_pattern) |
There was a problem hiding this comment.
Minor inconsistency: The result of result.match() is a Matchobject, not a boolean. To be more precise:
has_url_prefix = re.match(HAS_URL_PREFIX, source_pattern) is not None| HAS_URL_PREFIX = re.compile(r"^(https?://)") | ||
| _ALLOWED_GIT_PLATFORMS = ["github.com", "bitbucket.org", "gitlab.com"] | ||
| GIT_REPO_REGEX = re.compile( | ||
| r"^(https?://|git@)({})/[\w-]+/[\w-]+".format("|".join(map(re.escape, _ALLOWED_GIT_PLATFORMS))) |
There was a problem hiding this comment.
Not sure of \w- is enough, there might be all kinds of cases where this could fail for legitimate URLs, e.g. when using URL escaping based on %. Maybe we should go with something more lenient, that would accept a few non-URLs. In that case, it will fail later anyway when trying to interact with the URL, which should cause some system function to fail and report a somewhat descriptive error message. Which would be good enough from my point of view.
Also, we should be able to avoid the format() using f-strings. How about this?
_ALLOWED_GIT_PLATFORM_CHOICES_PATTERN = "|".join(map(re.escape, _ALLOWED_GIT_PLATFORMS)
GIT_REPO_REGEX = re.compile(
fr"^(https?://|git@)({_ALLOWED_GIT_PLATFORM_CHOICES_PATTERN})/[^/]+/[^/]+"
)| def test_fails_on_non_git_urls(self): | ||
| non_repo_urls = [["https://no/git/url"], ["https://google.com/nogit"]] | ||
| for non_repo_url in non_repo_urls: | ||
| with analysis.SourceScanner(non_repo_url) as scanner, pytest.raises(pygount.Error): |
There was a problem hiding this comment.
Use pytest.raises(pygout.Error, match="...") to also check the error message. You can use a regex here.
If the expected error message is long, I typically only check the first part of the sentence.
I added the previous issue i contributed as well