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.

Sign up for GitHub

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

Jump to bottom

chore: simplify regex #276

Merged
merged 2 commits into from
Jun 25, 2024
Merged

chore: simplify regex #276

merged 2 commits into from
Jun 25, 2024

Conversation

msopacua
Copy link
Contributor

@msopacua msopacua commented May 2, 2024 •
edited
Loading

  • We already do toLowerCase(), so don't need i flag.
  • We're not using the http bit, so no need to capture it.
  • If there's an s, append it, if not don't. This is the same as just replacing http with ws:
    • [http]:// -> [ws]://
    • [http]s:// -> [ws]s://
    • [http]x:// -> [ws]x:// -> error. Is ok.
    • [http]sx:// -> [ws]sx:// -> error in both. Is ok.
  • We don't want to replace protocols that do not start with http, such as git+https, so we can anchor at string start and leave out the g flag.

Much simpler -:)

@msopacua
chore: simplify regex
fbcdee7 
- We already do toLowerCase(), so don't need i flag.
- We're not using the http bit, so no need to capture it.
- If there's an s, append it, if not don't. This is the same as just
  replacing http with ws:
  [http]:// -> [ws]://
  [http]s:// -> [ws]s://
  [http]x:// -> [ws]x:// -> error. Is ok.
  [http]sx:// -> [ws]sx:// -> error in both. Is ok.
- We don't want to replace protocols that do not start with http, such
  as git+https, so we can anchor at string start and leave out the g
  flag.

Much simpler -:)
@lukeocodes
Copy link
Contributor

Oh it's possible this is different as of #273, I can't remember if I replaced this code or moved it

@lukeocodes lukeocodes self-requested a review May 7, 2024 15:33
lukeocodes
lukeocodes approved these changes May 7, 2024
View reviewed changes
Copy link
Contributor

@lukeocodes lukeocodes left a comment

Choose a reason for hiding this comment

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

approved - but will wait for 3.4

@lukeocodes lukeocodes merged commit a47953a into deepgram:main Jun 25, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukeocodes lukeocodes lukeocodes approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@msopacua @lukeocodes