Some users prefer to create revisions using raw diffs. This workflow is convenient because it does not require installation of arcanist. Users are required to specify the repository for which this diff is intended.
Unfortunately Revisions created with such method have a serious drawback - very few unchanged lines are visible, so maintainers can't gather the context of the change. This creates a need for a maintainer to have a full source code of a file open somewhere to review the change. We often end up asking our contributors to regenerate the diff with git diff -U99999 so maintainers can see the entire file.
Given that users specify the repository and diff contains file names - it seem that it should be possible for Phorge to show all surrounding unchanged lines without needing to do git diff -U99999
Secondly, if this is really impossible to do we should at least show users correct commands. The tooltip at https://reviews.freebsd.org/differential/diff/create says:
You can also paste a diff above, or upload a file containing a diff (for example, from svn diff, git diff or hg diff --git).
where we should at least be saying something like:
You can also paste a diff above, or upload a file containing a diff (for example git diff -U9999). Pls-pls make sure your patch has as many unchanged lines as possible..
These are the context buttons that are not shown in Differentials when pasting a git diff:
Trying to understand what happens
This is simply an attempt to understand the current situation to help future hackers:
- the rendered of the context buttons is this class: DifferentialChangesetHTMLRenderer method renderShowContextLinks()
- the method renderShowContextLinks() generates the buttons but it's not called in our case
- the method DifferentialChangesetHTMLRenderer#renderShowContextLinks() in normal conditions is probably called from class DifferentialChangesetTwoUpRenderer, method renderTextChange()
- the problem seems: $mask[$ii] must be empty, in order to call DifferentialChangesetHTMLRenderer#renderShowContextLinks()
- $mask is taken from DifferentialChangesetRenderer#getMask()
- its value is taken from DifferentialChangesetRenderer#setMask()
- the caller of setMask() is in class DifferentialChangesetParser.php line 1168
- the $mask in short is calculated from DifferentialChangesetParser.php method calculateGapsAndMask()
- (... and you need the $gaps - calculated in the same method)
- → so probably this is a good candidate to be somehow changed: calculateGapsAndMask(): https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php;a5c93dea568bcdf59122e52bd8bd5e8c849bcfd7$1179-1234
- the problem seems: $mask[$ii] must be empty, in order to call DifferentialChangesetHTMLRenderer#renderShowContextLinks()
/** * This function calculates a lot of stuff we need to know to display * the diff: * * Gaps - compute gaps in the visible display diff, where we will render * "Show more context" spacers. If a gap is smaller than the context size, * we just display it. Otherwise, we record it into $gaps and will render a * "show more context" element instead of diff text below. A given $gap * is a tuple of $gap_line_number_start and $gap_length. * * Mask - compute the actual lines that need to be shown (because they * are near changes lines, near inline comments, or the request has * explicitly asked for them, i.e. resulting from the user clicking * "show more"). The $mask returned is a sparsely populated dictionary * of $visible_line_number => true. * * @return array($gaps, $mask) */ private function calculateGapsAndMask( ... }
Also note:
- the "Show more" frontend action seems to be initially handled by this code: https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php;a5c93dea568bcdf59122e52bd8bd5e8c849bcfd7$1291-1327
