Skip to content

refactor: disjoint parameter types for commits vs text search

Created by: rvantonder

Stacked on #7492.

The TextParameters data type currently bundles a lot of state for all kinds of search (text, commit, diff) etc. The search_results.go logic cases out on the kind of search, passing all of this state down, which is then converted later into the other types like CommitParameters, DiffParameters, etc. that I've pulled out prior.

The intent with this PR, and other follow ups, is to disentangle the state that is currently encoded in TextParameters (it is basically a dumping ground currently). To disentangle this safely, I am introducing temporary intermediate data types that exclude the struct members that are being threaded along and not needed. This PR introduces a temporary type that converts TextParameters -> TextParametersForCommitParameters. The verbose naming can change later. This type excludes the Zoekt and SearcherURLs, and other things that are simply not used for commit search.

It is simply too error prone to try and rewrite the logic without introducing intermediate types. These types help prop up temporary barriers for bundling irrelevant state, and once the intermediate types are in place, it's easier to create disjoint types that are further bundled inside these.

What I really want to do is fix up PatternInfo for textsearch by creating disjoint types, but this isn't possible without separating its uses from commit search that is currently entangled in other places.

To understand the problem down the line, consider that, e.g., CommitParameters currently stores both PatternInfo.Pattern and git.TextSearchOptions.Pattern (these members are either the same, and if they are not, it could lead to some broken invariants). We are also threading Query along everywhere which is sloppy (cf. comment note in TextParameters). Further, the IsCaseSensitive member is accessible in three ways inside search.CommitParameters: through TextSearchOptions, Query, and PatternInfo.Info, which is just, yeah... it's impossible to know which one is used (or should be used) without looking at the code. But the real issue is that underneath the hood, the PatternInfo type contains a lot of information that is (a) irrelevant to other kinds of searchers and (b) should itself be separated because it is also a 'dumping ground' for text search logic. This PR is just prep work for that end goal.

Merge request reports

Loading