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

set correct groups indices to extract author name and email #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stijnster
Copy link

Hi,

I stumbled upon your script and found it very useful to integrate local hosted repositories with my online time tracking which is compatible with github style webhooks, so thanks for your work on that!!

I did notice that in the webhook json, the author name was empty and the mail field contained the author name.

	"commits": [
		{
			"id": "880d1cbc6dcdd811c5be441896b37e6192cd1d88",
			"author": {
				"name": "",
				"email": "Stijn Mathysen"
			},
			"url": null,

I'm no expert in python, but I noticed you use regional expressions to extract both name and email;

            props['name'] = m.group(1)
            props['email'] = m.group(2)
EMAIL_RE = re.compile(r"^(\"?)(?P<name>.*)\1\s+<(?P<email>.*)>$")

Your regional expression however, defines three groups, the first being (optional) quotes, then the name and then the e-mail.

Taking the initial group into account, and moving the indexes solved my issue and now both author name and e-mail come through.

This pull requests fixes that index offset issue. I hope it's useful to you.

Kind regards,

Stijn

@robbat2
Copy link
Collaborator

robbat2 commented Apr 23, 2021

Hi!

Sorry for the delay, could you tweak your patch to use the named regex groups 'name' and 'email' and then I'll merge it? That way this code will last longer after further EMAIL_RE improvements. Alternatively, this block could use the extract_name_email function now.

@robbat2
Copy link
Collaborator

robbat2 commented May 1, 2024

@stijnster bump on that improvement?

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.

2 participants