Skip to content

Conversation

@kingcanfish
Copy link
Contributor

@kingcanfish kingcanfish commented Oct 17, 2024

fix(tree): Keep panic infos consistent when wildcard type build faild

When I run the demo1

// demo1
func main() {
	router := gin.Default()
	router.GET("/abc/bar", nil)
	router.GET("/abc/x*z", nil)

	router.Run("localhost:8080")
}

it got the panic:panic: no / before catch-all in path '/abc/x*z'

but when i run the demo2

// demo2
func main() {
	router := gin.Default()
	router.GET("/abc/bar", nil)
	router.GET("/abc/b*r", nil)

	router.Run("localhost:8080")
} 

it got the panic: panic: runtime error: index out of range [-1]
Although the end result is the same(panic), the different panic informations may cause some confusion.

So, add the i < 0 condition to keep the panic infos same

@kingcanfish
Copy link
Contributor Author

#4066 This PR fixed the ci-lint faild code

@appleboy
Copy link
Member

build failed

@appleboy
Copy link
Member

waiting for #4081

@appleboy appleboy added this to the v1.11 milestone Oct 25, 2024
@appleboy
Copy link
Member

please update the branch to the latest version.

@kingcanfish
Copy link
Contributor Author

please update the branch to the latest version.

thank you for your review. I have updated to the latest version, but the golangci-lint still fails.

in my machine, golangci-lint (v1.60.2) is success, the same version as on github(v1.58.1) failed to run,
so may be we need update the lint to the latest version?

@codecov
Copy link

codecov bot commented Oct 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.96%. Comparing base (3dc1cd6) to head (26f5476).
Report is 78 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4077      +/-   ##
==========================================
- Coverage   99.21%   98.96%   -0.25%     
==========================================
  Files          42       44       +2     
  Lines        3182     3478     +296     
==========================================
+ Hits         3157     3442     +285     
- Misses         17       25       +8     
- Partials        8       11       +3     
Flag Coverage Δ
?
-tags "sonic avx" 98.95% <100.00%> (?)
-tags go_json 98.95% <100.00%> (?)
-tags nomsgpack 98.95% <100.00%> (?)
go-1.18 ?
go-1.19 ?
go-1.20 ?
go-1.21 98.96% <100.00%> (-0.25%) ⬇️
go-1.22 98.96% <100.00%> (?)
macos-latest 98.96% <100.00%> (-0.25%) ⬇️
ubuntu-latest 98.96% <100.00%> (-0.25%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kingcanfish
Copy link
Contributor Author

please update the branch to the latest version.

thank you for your review. I have updated to the latest version, but the golangci-lint still fails.

in my machine, golangci-lint (v1.60.2) is success, the same version as on github(v1.58.1) failed to run, so may be we need update the lint to the latest version?

update the golangci-lint to v1.62.0 :)

@appleboy appleboy added the type/bug Found something you weren't expecting? Report it here! label Oct 26, 2024
@appleboy appleboy merged commit ea53388 into gin-gonic:master Oct 26, 2024
24 of 25 checks passed
@appleboy
Copy link
Member

@kingcanfish Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement type/bug Found something you weren't expecting? Report it here!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants