We currently allow users to get links to line numbers in Diffusion in file view, but not in a Revision or a Commit. This is a little inconsistent.
Description
Event Timeline
This is fairly involved in the general case:
- Clicking lines already has an effect: creating an inline comment. That seems like the correct effect, given that we've rarely/never seen requests for line anchors. So we need some other interaction? Or a separate anchor link next to every line?
- Changes and sections of changes may be hidden by default:
- Generated changes hide file content by default.
- Commits/revisions affecting more than 100 (or 250 or whatever?) files hide all file content by default.
- Lines which are part of the unchanged context are hidden by default.
- We solve some of this already with inline comments and table-of-contents anchors, but that problem is less complicated because we alway reveal context around inlines on the server, so it's not possible to have an inline hidden behind "Show All 67 Lines of Context". We can't reveal context around inlines on the server for every possible linked line since that would defeat the purpose of folding anything.
So this is probably several hours worth of Javascript, and I'm not sure what the interaction should look like.
Looks like we could do what we do in Diffusion already, just change the URL, no interaction needed inline?
We could change the URL as a side effect of starting an inline comment, although you'd have to cancel the inline comment afterward.
The second bullet point is the bigger problem. To reproduce the issue, do this:
- Go here: https://secure.phabricator.com/rP7126025fe637a4d931645f05f9ad125381600755
- Scroll to PhabricatorProjectMembersAddController.php.
- Click "Show All 35 Lines" to reveal the hidden context.
- Imagine getting a line anchor for line 70, so the URL now theoretically reads something like rP7126025fe637a4d931645f05f9ad125381600755#PhabricatorProjectMembersAddController.php-line70.
Now, reload the page.
Here's the problem: line 70 does not exist on the page right now.
We need to parse the anchor, figure out which changeset needs to expand, which section of it needs to expand, how much of it it needs to be loaded, then submit the request to fill it in, scroll the user to it when it comes back, and highlight it.
Now, imagine you didn't just click "Show All 35 Lines", but first clicked "This file is generated, and does not normally require detailed human review. Show Changes", then clicked "Show all 35 Lines".
Now, imagine you didn't just reveal the generated changes and show context, but also clicked "This large change affects more than 250 files. Individual changes are hidden by default. Show All".
The Javascript needs to be able to parse the anchor and page state and unroll it through all the possible shields/folds, and carry the "we are jumping to a line anchor" logic through all of that.
It looks like the original user may have just wanted (or been satisfied with) a Diffusion file-line link anyway? Thread is sort of ambiguous:
https://twitter.com/timonus/status/738121400368521218
I'm also not entirely sure what the use case for linking to lines in diffs (vs leaving inline comments) is.