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

Fix regex for validating mirror remoteUrl #1029

Merged
merged 2 commits into from
Aug 28, 2024
Merged

Conversation

minwoox
Copy link
Member

@minwoox minwoox commented Aug 28, 2024

Motivation:
The regex for validating mirror remoteUrl is wrong. [.-\/] is interpreted as . to /. However, it should be [.\-\/] which allows ., - and / characters.

Modifications:

  • Fixed regex for validating mirror remoteUrl

Result:

  • You can now correctly setup a mirror url when it contains - in its name.

Motivation:
The regex for validating mirror remoteUrl is wrong.
`[.-\/]` is interpreted as `.` to `/`. However, it should be `[.\-\/]` which allows `.`, `-` and `/` characters.

Modifications:
- Fixed regex for validating mirror remoteUrl

Result:
- You can now correctly setup a mirror url when it contains `-` in its name.
Copy link
Contributor

@jrhee17 jrhee17 left a comment

Choose a reason for hiding this comment

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

👍 👍 👍

Copy link
Contributor

@ikhoon ikhoon left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -300,7 +300,7 @@ const MirrorForm = ({ projectName, defaultValue, onSubmit, isWaitingResponse }:
type="text"
defaultValue={defaultValue.remoteUrl}
placeholder="my.git.com/org/myrepo.git"
{...register('remoteUrl', { required: true, pattern: /^[\w.-]+(:[0-9]+)?\/[\w.-\/]+.git$/ })}
{...register('remoteUrl', { required: true, pattern: /^[\w.-]+(:[0-9]+)?\/[\w.\-\/]+.git$/ })}
Copy link
Contributor

Choose a reason for hiding this comment

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

😅

Copy link
Contributor

Choose a reason for hiding this comment

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

By the way, do we have to escape the first -?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have to because - is in the last position in []. However, let me add it for clarification.

@minwoox minwoox merged commit 0d365bd into line:main Aug 28, 2024
9 of 10 checks passed
@minwoox minwoox deleted the fix_regex branch August 28, 2024 04:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants