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

Refactor strip_web_protocol() #126

Merged
merged 2 commits into from
May 4, 2024
Merged

Conversation

JinguBangWest
Copy link
Contributor

This allows for more prefixes to be added in the future if needed without repeating code multiple times.

This allows for more prefixes to be added in the future if needed without repeating code multiple times.
prefixes = ['https://','http://','www.','www2.']
for prefix in prefixes:
if string.startswith(prefix):
string = string.replace(prefix, '')
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider following example:

>>> 'www.foowww.com'.replace('www.', '')
'foocom'

Copy link

Choose a reason for hiding this comment

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

This should handle it. Basically replacing only the first occurance of the prefix

>>> 'www.foowww.com'.replace('www.', '', 1)
'foowww.com'

Copy link
Collaborator

Choose a reason for hiding this comment

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

...why not keep it simple?

-       if string.startswith(prefix):
-            string = string.replace(prefix, '')
+       string = string.removeprefix(prefix)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I meant to leave it as string.removeprefix() but didn't change it back for my commit. Also @yedpodtrzitko your right we should keep it simple. I've went ahead and changed this in my latest commit.

@Loran425 Loran425 added the Type: Refactor Code that needs to be restructured or cleaned up label May 4, 2024
@Loran425
Copy link
Collaborator

Loran425 commented May 4, 2024

Functionally identical, looks good.

@CyanVoxel CyanVoxel merged commit ac3d7c9 into TagStudioDev:main May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Refactor Code that needs to be restructured or cleaned up
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants