Closed Bug 1601715 Opened 5 years ago Closed 2 years ago

The order of "Go to definition" is not intuitive

Categories

(Webtools :: Searchfox, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 1776522

People

(Reporter: jrmuizel, Unassigned)

Details

Clicking on CreateMaskLayer here https://searchfox.org/mozilla-central/source/layout/painting/FrameLayerBuilder.cpp#5841

Gives a menu like this:

Go to definition of RefPtr::RefPtr<T>
Go to definition of mozilla::ContainerState::CreateMaskLayer
Search for constructor RefPtr::RefPtr<T>
Search for function mozilla::ContainerState::CreateMaskLayer
Search for the substring CreateMaskLayer
Sticky highlight

It would be better if Go to definition of mozilla::ContainerState::CreateMaskLayer
was above Go to definition of RefPtr::RefPtr<T>

I think the current ordering is just that of the analysis file, the excerpts for which currently look like this in its on-disk order, noting that when we ingest the records with read_analysis we sort by loc before de-duping. I'm including line-numbers here because again, the on-disk ordering is somewhat non-intuitive[1].

grep -n 5841:10 FrameLayerBuilder.cpp.json
5466:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"RefPtr::RefPtr<T>","sym":"_ZN6RefPtrC1EO16already_AddRefedIT_E","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
5467:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
7466:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeImEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
8831:{"loc":"05841:10-25","target":1,"kind":"use","pretty":"mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIyEE","context":"mozilla::ContainerState::SetupScrollingMetadata","contextsym":"_ZN7mozilla14ContainerState22SetupScrollingMetadataEPNS_13NewLayerEntryE"}
17920:{"loc":"05841:10-25","source":1,"syntax":"constructor,use","pretty":"constructor RefPtr::RefPtr<T>","sym":"_ZN6RefPtrC1EO16already_AddRefedIT_E"}
17921:{"loc":"05841:10-25","source":1,"syntax":"function,use","pretty":"function mozilla::ContainerState::CreateMaskLayer","sym":"_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE,_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeImEE,_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIyEE"}

The logic in the source code is at https://github.com/mozsearch/mozsearch/blob/cf247e4b010b5da900345e253e105e7410c866ac/tools/src/format.rs#L200 where you can see 2 passes. In all cases, we're processing "source" records from d.

  • menu_jumps is processing the jumps data which comes from "target" records.
  • items is just from those "source" records in d.

The jumps entries in question are:

["_ZN6RefPtrC1EO16already_AddRefedIT_E","mfbt/RefPtr.h",120,"RefPtr::RefPtr<T>"]
["_ZN7mozilla14ContainerState15CreateMaskLayerEPNS_6layers5LayerERKNS_15DisplayItemClipERKNS_5MaybeIjEE","layout/painting/FrameLayerBuilder.cpp",7405,"mozilla::ContainerState::CreateMaskLayer"]

While we have limited ability to order on the information here, in my graph-support branch I track more per-symbol metadata and we can maybe do something more semantic than just longest string or counting the namespace delimiters.

1: C++ analysis files undergo 2 orderings in their lives:

Uh, and I mention all this in case you want to try your hand at a fix. Although I mention the graph-support branch, it's far from imminent. https://github.com/mozsearch/mozsearch has (just updated!) docs on how to set up searchfox in a VM (no longer requires virtualbox!). If the existing test corpus doesn't have coverage for this case, it should be reasonably feasible to add one. (I can also set you up with AWS creds if you want to do dev pushes to iterate once you've gotten things looking okay locally on the test corpus.)

I'm fixing this in bug 1776522 now as we now will make all decisions about menu ordering in the JS content logic, although it's possible I'll bungle the heuristics!

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → DUPLICATE
You need to log in before you can comment on or make changes to this bug.