feature: it works, is ugly

[alvarmaciel]

Add the ability to download from any public github URL

[github-actions]

Coverage Report

File	Stmts	Miss	Cover	Missing
shbin.py	147	15	89%	65–66, 76, 97–98, 101, 139–141, 151, 216–217, 224–225, 256
TOTAL	147	15	89%

Tests	Skipped	Failures	Errors	Time
46	0 💤	0 ❌	0 🔥	8.639s ⏱️

[mgaitan]

Here e in the message is not defined.

Also, it's considered more pythonic the EAFP approach

So, it should something like

Suggested change

	result = re.findall(pattern, url)
	if result:
	extracted_string = result[0]
	return extracted_string
	else:
	raise DocoptExit(f"Ensure your path is from github repository. (error {e})")
	try:
	return re.findall(pattern, url)[0]
	except TypeError:
	raise DocoptExit(f"Ensure the URL is from github repository")

[mgaitan]

do not use findall for a single potential match. also it would be a little better to compile the pattern at module level

PATTERN = re.compile(r"https://github.com/.*?/(?:tree|blob)/(.*)")

def get_path(url):
    try:
         return re.match(pattern, url).group(1)
    except ...     

[mgaitan]

a few things here.

• the first condition should be not url_or_path.startswith("https://")

• is it enough to assume that if it doesn't start with https:// then is from repo?

• wording: "my_repo" . I'm the user but the repo could someone else and I just have permissions, right? like mgaitan downloading from Shiphero/pastebin. I would use is_from_the_repo or something like that

[mgaitan]

if we are trying to parse with a regex, couldn't we generalize it an accept urls or user/repo/path forms for any repo and then check if it's the our not?

suppose shbin zauberzeug/nicegui/examples

[mgaitan]

what if we unify own or foreign contents with requests? the only difference it that you need to pass the token in the header.

In [16]: requests.get("https://raw.githubusercontent.com/Shiphero/pastebin/main/mgaitan/15e1VFq87UA.txt",
    ...:     headers={"Authorization": f"token {os.environ['SHBIN_GITHUB_TOKEN']}", "Accept": "application/vnd.github.v3.raw'"}
    ...: )
Out[16]: <Response [200]>

In [17]: Out[16].content[:144]
Out[17]: b"    \n>       mock_sns.push.assert_called_once_with(topic_arn, message)\nE       AssertionError: expected call not found.\nE       Expected: push('"

the good news is that passing a token to a public content doesn't affect, so this could be the only code

In [20]: requests.get("https://raw.githubusercontent.com/facebook/react/main/README.md",
    ...:     headers={"Authorization": f"token {os.environ['SHBIN_GITHUB_TOKEN']}", "Accept": "application/vnd.github.v3.raw'"}
    ...: )
Out[20]: <Response [200]>

[mgaitan]

which other exception could be?

[mgaitan]

this has no sense Alvar. It's a self-fulfilling prophecy: "I will prove that the following code raise an exception " and the following code is... literally raise Exception().

[mgaitan]

we don't need a subfolder. tmp_path is already a temporary Path instance.

[mgaitan]

not a good practice to change the dir in this way. When needed, use monkeypatch.chdir so it's undo automatically on teardown (even if the test fails or errored). https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.MonkeyPatch.chdir

[mgaitan]

it would nice to have an option to define where to download the contents, like

shbin dl URL  [-o  path]