1466692 - Many irrelevant results included in this search

Status: RESOLVED FIXED

(Webtools :: Searchfox, enhancement)

Description

[Markus Stange [:mstange]]

A search for nsdisplayoutline::creat gives me 85 results:

https://searchfox.org/mozilla-central/search?q=nsdisplayoutline%3A%3Acreat&path=

Only one of those results is nsDisplayOutline::CreateWebRenderCommands, which is the one I'm interested in. All the other results have nothing to do with nsDisplayOutline.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

I think the basic issue is that the identifiers table for your symbol looks like this:

nsDisplayOutline::CreateWebRenderCommands _ZN13nsDisplayItem23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder
nsDisplayOutline::CreateWebRenderCommands _ZN16nsDisplayOutline23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder

The first one is for DisplayItem and not what you want, the second one is for DisplayOutline and is what you want.  identifier_search at https://github.com/mozsearch/mozsearch/blob/master/router/router.py#L253 "helpfully" puts them all together in the same big bag.  I think this behavior probably makes sense for overloaded symbols that reduce to the same pretty symbol like "nsDocShell::Stop(unsigned int)" "nsDocShell::Stop()" do, but not for "nsIWebNavigation::Stop(unsigned int)" that also has an identifiers entry for "nsDocShell::Stop".  Ordering could be based on longest case-insensitive prefix-match.


If you do a "symbol:" search with the mangled symbol, that bypasses the indirection and you're fine.  For this case, that's:
https://searchfox.org/mozilla-central/search?q=symbol%3A_ZN16nsDisplayOutline23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder&path=

Of course, there's no real way to figure that out as a human.  The context-menu in the source view populated by ANALYSIS_DATA only has a single entry that has *both* symbols present, comma-delimited.  I believe that's because we're directly translating the "source" record that has them like that already in the "sym" line for overrides at https://github.com/mozsearch/mozsearch/blob/master/tools/src/format.rs#L140 for this data:
{"loc":"04928:18-41","source":1,"syntax":"def,function","pretty":"function nsDisplayOutline::CreateWebRenderCommands","sym":"_ZN16nsDisplayOutline23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder,_ZN13nsDisplayItem23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder"}

Note that the "target" records for the definition are nicely separate, but not relevant at the point of definition (the jumps are suppressed):
{"loc":"04928:18-41","target":1,"kind":"def","pretty":"nsDisplayOutline::CreateWebRenderCommands","sym":"_ZN13nsDisplayItem23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder"}
{"loc":"04928:18-41","target":1,"kind":"def","pretty":"nsDisplayOutline::CreateWebRenderCommands","sym":"_ZN16nsDisplayOutline23CreateWebRenderCommandsERN7mozilla2wr18DisplayListBuilderERNS1_22IpcResourceUpdateQueueERKNS0_6layers21StackingContextHelperEPNS6_21WebRenderLayerManagerEP20nsDisplayListBuilder"}

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

:mstange, my changes for bug 1641372 potentially result in a change in behavior here and I wanted to see your thoughts. There's more info in https://github.com/mozsearch/mozsearch/pull/408 but the summary is:

• The existing https://searchfox.org/mozilla-central/search?q=nsdisplayoutline%3A%3Acreat&path= has all of the overloaded superclass/overrides results you didn't want
• The new https://asuth.searchfox.org/mozilla-central/search?q=nsdisplayoutline%3A%3Acreat&path= has just the 1 result grouped by "Definitions (nsDisplayOutline::CreateWebRenderCommands)" because we don't walk up the superclass / overrides automatically.

Would you only want the new behavior in this case, or would you want some combination of:

• Showing the parent override (but not all of its other overrides)
  • In a separate group, so we have "Definitions (nsDisplayItem::CreateWebRenderCommands)" in addition to the existing "Definitions (nsDisplayOutline::CreateWebRenderCommands)"
  • By having the group header have some interactive pieces like "Definitions ([x] nsDisplayOutline::CreateWebRenderCommands, [ ] overridden nsDisplayItem::CreateWebRenderCommands, [ ] nsDisplayItem::CreateWebRenderCommands overrides)"
• Optionally showing all of those other overrides, possibly via a search upsell link that is just an entirely new search
• Something else!

Comment 3

[Markus Stange [:mstange]]

Great!

The new behavior would definitely work for me.
In addition, linking to separate searches for the superclass implementation and for the overrides would be nice but I don't need it very often.

If you really want to show information about nsDisplayItem::CreateWebRenderCommands on the same search page, then both the definition and the declarations of nsDisplayOutline::CreateWebRenderCommands should come first, above that additional section.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

With the landing of the structured analysis functionality (bug 1641372), I think this bug can be marked close per comment 3. (Noting that there is some fallout from the changes perhaps being too major, with bug 1729164 being the current open example of that.)

Comment 5

[Markus Stange [:mstange]]

Very nice! This works perfectly for my use case. Thank you!