Change Details

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: {F281286} == 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: [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php$88 | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php$1167-1169 | class DifferentialChangesetParser.php line 1168 ]] * the `$mask` in short is calculated from `DifferentialChangesetParser.php` method `calculateGapsAndMask()` * ...

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: {F281286} == 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: [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php$88 | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php$1167-1169 | 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: https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php;a5c93dea568bcdf59122e52bd8bd5e8c849bcfd7$1179-1234

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: {F281286} == 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: [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php$88 | 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 [[ https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php$1167-1169 | 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: https://we.phorge.it/source/phorge/browse/master/src/applications/differential/parser/DifferentialChangesetParser.php;a5c93dea568bcdf59122e52bd8bd5e8c849bcfd7$1179-1234