Open Bug 1776522 Opened 3 months ago Updated 2 months ago

Move from using ANALYSIS_DATA for context menus entirely to SYM_INFO (or new name) and stop generating ANALYSIS_DATA.

Categories

(Webtools :: Searchfox, enhancement)

enhancement

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: asuth, Assigned: asuth)

References

Details

I've discussed this in passing several times, but the basic situation is that currently context menus are powered by data-i attributes which provide a numeric index into the ANALYSIS_DATA array we build during HTML generation. Each entry in the ANALYSIS_DATA array is an array tuple of the form:

  1. jumps: An array listing known definitions relevant for the current token position of the form { sym, pretty, path } where path also includes a line number anchor on the end for the line containing the definition. Multiple definitions can be associated with a token resulting in multiple "go to def of" menu items. We now use direct links since a recent enhancement, but previously we bounced through the "define" CGI endpoint but that causes spurious page reloads for in-page navigations.
  2. searches: An array listing known symbols relevant for the current token position of the form { sym, pretty }. These always present as searches in the menu.

ANALYSIS_DATA is incredibly redundant. No attempt is made to de-duplicate entries in the array. There can be some level of variation between entries for the same token/symbol, but these amount to:

  • We don't emit a jump/definition on the line that we know is where the definition is, because there's no point going to the line you're already on.
  • Sometimes a token like a constructor ends up with a whole bunch of other stuff mis-attributed to that token, like implicit constructors.

Since the landing of the structured record support we actually do have a SYM_INFO dict already providing { syntax, type, typesym } meta-information for locals which are not available in the crossref database as an artifact of the ReactJS-based prototype fancy UI which will never land.

The plan here is:

  • Store all the information ANALYSIS_DATA currently provides in the SYM_INFO per-symbol map.
  • Populate the context menu by walking the list of data-symbols (which we already populate for use by our hover mechanism and the ReactJS UI knew how to consume) and lookups against SYM_INFO.
  • SYM_INFO is also able to store additional information like the structured record information for a symbol so that this can be used to show cool information in the super sidebar, or bootstrap "query" requests that the super sidebar issues.
    • We are not including the complete crossref information for a symbol, as this can be quite voluminous and we have no intent to surface the results in the current page. The idea is to have the structured info, the "pretty" independently for fallback when there's no structured info, plus any singleton def/decl/idl hits.
  • The SYM_INFO rep also allows for the "query" mechanism's dynamically generated graphs to have an identical interaction pattern.

New features that will be added during this process and will constitute a change in behavior:

  • We are no longer limited to a single "jump" per symbol which means that we can provide all of:
    • Go to IDL (currently this supersedes go to def)
    • Go to definition
    • Go to declaration. I've found this useful for C++ where I'd like to go to the header where the comments are (in the Fancy branch prototype UI). I could see including some kind of iconic badge on the decl (and def too) if we know there's a block comment associated with them, which would be the case if we have a peekRange with a first line before the canonical line of the def.

It's unclear what the size trade-off will be here, as there should be much less duplication, but there will be a net increase in data stored.

I may have discussed this previously in the context of also changing our blame lookups to avoid network requests in favor of having that data stored in the document as well in a JSON blob or templates or something, but that change won't happen as part of this change.

I've got the core of this implemented, but have run into bug 1727789 adjacent issues where the cleverness we added in bug 1759064 to add multiple "source" records knowing that they would translate directly into "search" directives in the ANALYSIS_DATA [jumps, searches] model is now causing problems.

My tentative plan is to:

  • Have idl-analyze.py emit a "structured" record.
  • Generalize the "structured" analysis record's "srcsym"/"targetsym" model to support the idea of named "slots" (or "relations"/"rel"s?) like I propose in bug 1727789 under the "IDL analyzer simplification" sub-heading.
  • The structured record will also identify the underlying language/scheme we're using, and for idl-analyze.py that will be "xpidl" which will mark a little bit of a transition for us as right now we do tend to refer to XPIDL as "idl" but I think we need to try and move away from that because it's confusing and we can benefit from having a more generic concept of IDL which isn't conflated with XPIDL.
  • context-menu.js for now will have all the UX smarts about how to make use of those slots to build the searches. The rationale is that:
    • This is the most obvious and easiest way for (potential) contributors to change searchfox's behavior.
    • The alternate approach would be to define some kind of JSON or TOML schema to map these and have the crossref process apply the schema to the slots when building the database, but unless the server needs to uniformly apply the same logic, this feels more like obfuscation than being helpful.
      • Right now the server doesn't need to know these things because the "search" endpoint just takes the list of symbols to search on and loses the semantic concept that the menu was getting at. We can change this in the future with the "query" endpoint because it seems like most people don't get what's going on with the resulting "search" spec and how it can be manipulated, but that's further down the road when the "query" endpoint becomes usable.
Duplicate of this bug: 1601715
You need to log in before you can comment on or make changes to this bug.