Found while tracing where per-result state lives, for the chart-configuration fix in #2222.
ResultSet declares three pieces of state that nothing reads:
var sortState = SortState()
var pagination = PaginationState()
var columnLayout = ColumnLayoutState()
The live copies of all three are on QueryTab, which is why they survive TabDisplayState.replaceUnpinnedResults(with:): they sit outside display. A grep for reads through a ResultSet returns nothing for any of the three.
This is worth removing rather than leaving, because the duplicate is actively misleading. #2222 originally put chartConfiguration on ResultSet next to these three, which looked like the established place for per-result view state. It is not: ResultSet is rebuilt with a fresh UUID on every table load, page change, sort change and re-execute (QueryExecutionCoordinator+Helpers.swift:207 and three other callers), so anything stored there is discarded on a page turn. That is exactly the bug the review caught, and the dead properties are what made the wrong place look right.
What to do: delete the three properties, or, if they are scaffolding for per-result-tab sort and pagination, say so in a comment naming the intended feature. Right now there is nothing to tell a reader which it is.
Verified: zero readers on main at edae33c.
Found while tracing where per-result state lives, for the chart-configuration fix in #2222.
ResultSetdeclares three pieces of state that nothing reads:The live copies of all three are on
QueryTab, which is why they surviveTabDisplayState.replaceUnpinnedResults(with:): they sit outsidedisplay. A grep for reads through aResultSetreturns nothing for any of the three.This is worth removing rather than leaving, because the duplicate is actively misleading. #2222 originally put
chartConfigurationonResultSetnext to these three, which looked like the established place for per-result view state. It is not:ResultSetis rebuilt with a fresh UUID on every table load, page change, sort change and re-execute (QueryExecutionCoordinator+Helpers.swift:207and three other callers), so anything stored there is discarded on a page turn. That is exactly the bug the review caught, and the dead properties are what made the wrong place look right.What to do: delete the three properties, or, if they are scaffolding for per-result-tab sort and pagination, say so in a comment naming the intended feature. Right now there is nothing to tell a reader which it is.
Verified: zero readers on
mainatedae33c.