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

Fix whitespace being matched on commit cross-referencing #22668

Closed
wants to merge 1 commit into from
Closed

Fix whitespace being matched on commit cross-referencing #22668

wants to merge 1 commit into from

Conversation

yardenshoham
Copy link
Member

@yardenshoham yardenshoham commented Jan 30, 2023 •
edited

Changed the regex so it won't match whitespace

Screenshots

Source:

hi abc/fed@12d90af2cfe5500ec947bdefd237f2a8b6464b95 bye

Before

image

After

image

Fixes #22666

@yardenshoham
Fix whitespace being matched on commit cross-referencing
67ec40b 
Changed the regex and removed whitespace

Signed-off-by: Yarden Shoham <hrsi88@gmail.com>
@yardenshoham yardenshoham marked this pull request as draft January 30, 2023 21:21
@yardenshoham yardenshoham added this to the 1.19.0 milestone Jan 30, 2023
@yardenshoham
Copy link
Member Author

It's set as draft because I'm not sure what needs to be changed in the unit tests

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 30, 2023
@KN4CK3R
Copy link
Member

KN4CK3R commented Jan 30, 2023

I wonder why this works with all other patterns in this file. I copied the pattern issueAlphanumericPattern and modified it.

@yardenshoham
Copy link
Member Author

After the change I made, it's matching commits with more than 40 characters so not very good

@yardenshoham
Copy link
Member Author

Seeing as I'm not that good with regex I'll close this PR and wish luck to whoever is brave enough to go deep into regex territory.

@wxiaoguang
Copy link
Contributor

The regex is right.

What you need is:

RefLocation: &RefSpan{Start: m[2], End: m[7]},

@wxiaoguang wxiaoguang mentioned this pull request Jan 31, 2023
Merged
@wxiaoguang
Copy link
Contributor

The fix: Use correct captured group range when parsing cross-reference #22672

KN4CK3R added a commit that referenced this pull request Jan 31, 2023
@wxiaoguang @lunny
@KN4CK3R @zeripath
Use correct captured group range when parsing cross-reference (#22672)
36dc118 
Fixes #22666 (Replace #22668)

Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com>
Co-authored-by: KN4CK3R <admin@oldschoolhack.me>
Co-authored-by: zeripath <art27@cantab.net>
@lunny lunny removed this from the 1.19.0 milestone Jan 31, 2023
@yardenshoham yardenshoham deleted the whitespace branch January 31, 2023 11:50
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KN4CK3R KN4CK3R Awaiting requested review from KN4CK3R

Assignees
No one assigned
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cross-repo commit reference gobbles up whitespace
5 participants
@yardenshoham @KN4CK3R @wxiaoguang @lunny @GiteaBot