Refactor code_indexer to use an SearchOptions struct for PerformSearch #29724
Conversation
| func PerformSearch(ctx context.Context, repoIDs []int64, language, keyword string, page, pageSize int, isFuzzy bool) (int, []*Result, []*internal.SearchResultLanguages, error) { | ||
| if len(keyword) == 0 { | ||
| func PerformSearch(ctx context.Context, opts *SearchOptions) (int, []*Result, []*SearchResultLanguages, error) { | ||
| if opts == nil || len(opts.Keyword) == 0 { |
There was a problem hiding this comment.
I do not think opts == nil makes sense, how could it be nil? If "nil" opts means a bug, it should be handled in development stage.
There was a problem hiding this comment.
It should not be nil ... but better not panic
There was a problem hiding this comment.
It should not be nil ... but better not panic
If it shouldn't be nil, why not report the errors in development stage? Just by a panic.
I really dislike hiding errors. It makes it more difficult to debug some bugs.
There was a problem hiding this comment.
NO we should not panic if we can arange it. To hope for stacktraces to get reported as "linting" methode is wrong !!!
But what we can do is create a proper error to check against
There was a problem hiding this comment.
NO, why it would panic if there is no bug in code?
Golang library itself panics a lot if something is totally wrong.
There was a problem hiding this comment.
I also don't think we should check opts == nil here. It should not be nil if our code is right. Otherwise, you need to check another thousand places.
There was a problem hiding this comment.
Let me check (again) what comsumers this func has ... and also introduce a proper error instead of a ignore on keyword==""
There was a problem hiding this comment.
Hmm ... I only mean opts == nil check doesn't make sense.
if keyword == "" looks good to me, I do not think it's worth to introduce to many "errors".
Golang has a very bad error system, TBH I have been tired of writing a lot of if err != nil { ctx.ServerError }
There was a problem hiding this comment.
Since it has been merged, I think it could be left there as-is, no good no harm.
* giteaofficial/main: (28 commits) Forbid jQuery `.prop` and fix related issues (go-gitea#29832) Fix wrong test for TestPullView_CodeOwner (go-gitea#29838) Forbid HTML injection using jQuery (go-gitea#29843) Meilisearch double quote on "match" query (go-gitea#29740) Forbid variables containing jQuery collections not having the `$` prefix (go-gitea#29839) Remove AddParamIfExist(AddParam) (go-gitea#29841) Refactor markdown attention render (go-gitea#29833) Refactor code_indexer to use an SearchOptions struct for PerformSearch (go-gitea#29724) Refactor AddParam to AddParamIfExist (go-gitea#29834) Forbid jQuery AJAX (go-gitea#29818) Remove jQuery AJAX from the notifications (go-gitea#29817) Light theme color enhancements (go-gitea#29830) Better highlighting of archved labels (go-gitea#29749) Remove the `time-since` class (go-gitea#29826) Remove jQuery AJAX from the project page (go-gitea#29814) Upgrade `htmx` to v1.9.11 (go-gitea#29821) Dark theme color enhancements (go-gitea#29822) Remove jQuery AJAX from the comment edit box (go-gitea#29812) Tweak labeler (go-gitea#29809) Fix `for` attribute not pointing to the ID of the color picker (go-gitea#29813) ... # Conflicts: # routers/web/user/home.go
go-gitea#29724) similar to how it's already done for the issue_indexer --- *Sponsored by Kithara Software GmbH*
similar to how it's already done for the issue_indexer
Sponsored by Kithara Software GmbH