Go to definition (in the same file) should just be a direct link with line anchor rather than bounce through the `/TREE/define` CGI
Categories
(Webtools :: Searchfox, enhancement)
Tracking
(Not tracked)
People
(Reporter: asuth, Assigned: asuth)
References
(Blocks 1 open bug)
Details
The searchfox experience on large files can be sub-optimal when using "go to definition" menu options because the URL we use is to the /define server CGI endpoint rather than a direct link to the source file and this can result in the source file having to be reloaded when all that was required was scrolling.
We actually already have the necessary information available in the jumps file we use to power the choice[1] to "go to definition", but we only use it to filter out "go to definition" from the line that is itself the definition.
The only real upside to the current approach is that the /define URL could arguably serve as a stable semantic link for the symbol in question. However, I don't think I've ever really seen people use that URL, instead opting for the post-redirect actual source file location they get from following the link. Arguably if we want to provide a more stable semantic link, we should be providing that from the source listing page instead (and we have some bugs that consider this). The link does potentially work slightly better when the searchfox index has refreshed and the underlying definition location has changed, but the user experiencing a transition in revisions that they're looking at where the code being traversed has changed is arguably just a generally bad situation.
If we think it's worth sticking with /define, we could specialize things so that in the same-file case we use a direct file link, but I would argue that we might as well just go with the direct link in all cases. This could also be a configuration option, as the menu is dynamically generated from JSON data we embed in the formatted output.
1: Note that we may obsolete the jumps file in favor of having the formatting logic be able to directly consult the crossref database, but in that case we'll still have access to that information and potentially even do more clever things like handle the case where there are 2 definitions instead of giving up and forcing the user to perform a search instead.
|
Assignee | |
Comment 1•3 years ago
|
||
This is now in mozilla-central and will take effect on other indexers as they are run. I made this change as part of the fix for bug 1758394 and specifically in https://github.com/mozsearch/mozsearch/commit/03535421c7169fc65d1d536040f94fe7d652595a. Go to definition is now a direct link and this will make rlbox C files much happier to use when using "go to definition" because it'll just be a direct traversal in the same file without any navigation proper. (More specifically, hashchange will fire and stuff will happen.)
I think in my testing I might have somehow seen a case where the context menu didn't go away, so... we should be on the look out for regressions.
Description
•