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

Adding DigitalSpy #2312

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

Conversation

SOGeKING-NUL
Copy link

added support for the Digital Spy Forum. Digital Spy is a UK-based entertainment and media news website, known for covering television, movies, and celebrity news, as well as offering discussion forums for users.

@SOGeKING-NUL
Copy link
Author

@ppfeister this is my first time contributing, please tell me if the code needs changes

"errorType": "message",
"url": "https://forums.digitalspy.com/profile/{}",
"urlMain": "https://forums.digitalspy.com/",
"username_claimed": "papita69"
Copy link
Member

Choose a reason for hiding this comment

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

Looks like a good add! The only issue is that this username no longer appears to be active. When testing with blue however, it works as expected.

Probably worked fine when you added it, the account just closed down.

Suggested change
"username_claimed": "papita69"
"username_claimed": "blue"

Copy link
Author

Choose a reason for hiding this comment

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

noted,

"errorMsg": "The page you were looking for could not be found.",
"errorType": "message",
"url": "https://forums.digitalspy.com/profile/{}",
"urlMain": "https://forums.digitalspy.com/",
Copy link
Member

@ppfeister ppfeister Nov 1, 2024

Choose a reason for hiding this comment

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

It seems that DigitalSpy's username requirements are public:

Username can only contain letters, numbers, underscores, and must be between 3 and 20 characters long.

Could you add a regexCheck to match this? Not sure if you've dealt with regex before. I can provide a pattern for you if need be.

Note that this isn't a PROBLEM, just an improvement. It helps reduce traffic against any one target, which in turn reduces the odds of you getting rate limited.

Copy link
Author

Choose a reason for hiding this comment

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

I would very much like to add a regexCheck if you'd provide me with how to proceed with it :)

Copy link
Member

@ppfeister ppfeister Nov 4, 2024

Choose a reason for hiding this comment

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

Simple enough for this one, actually. The word character class \w just so happens to expand to exactly those characters [a-zA-Z0-9_] (letters, numbers, underscores). Pattern here for 3-20 chars would just be ^\w{3,20}$ Would need to escape the backslash in this case due to Python stuff: ^\\w{3,20}$. ^ for start of string and $ for end of string.

So "regexCheck": "^\\w{3,20}$" should be it, really.

Prefilled regex101 link if curious about the pattern: https://regex101.com/r/4fLLj0/1 (great tool for writing and testing patterns)

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