@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
When an inline was left on a rendered DocumentEngine document, don't include an email context patch
Summary:
Ref T13513. If you leave an inline on line 20 of a Jupyter document, we currently render context around *raw* line 20, which is inevitably some unrelated piece of JSON.
Instead, drop this context. (Ideal behavior would be to render context around Jupyter block 20, but that's a whole lot of work.)
Test Plan:
- On Jupyter changes and normal source changes, made and submitted inline comments, then viewed text and HTML mail.
- Saw no context on Jupyter comments (instead of bad context), and unchanged behavior (useful context) on normal source changes.
Maniphest Tasks: T13513
Differential Revision: https://secure.phabricator.com/D21242
+28
-6
+28
-6
src/applications/differential/mail/DifferentialInlineCommentMailView.php
+28
-6
src/applications/differential/mail/DifferentialInlineCommentMailView.php
···
59
59
$comment = $inline->getComment();
60
60
$parent_phid = $comment->getReplyToCommentPHID();
61
61
62
+
$inline_object = $comment->newInlineCommentObject();
63
+
$document_engine_key = $inline_object->getDocumentEngineKey();
64
+
62
65
$is_last_inline = ($inline_key == $last_inline_key);
63
66
64
67
$context_text = null;
···
70
73
$context_text = $this->renderInline($parent, false, true);
71
74
$context_html = $this->renderInline($parent, true, true);
72
75
}
76
+
} else if ($document_engine_key !== null) {
77
+
// See T13513. If an inline was left on a rendered document, don't
78
+
// include the patch context. Document engines currently can not
79
+
// render to mail targets, and using the line numbers as raw source
80
+
// lines produces misleading context.
81
+
82
+
$patch_text = null;
83
+
$context_text = $this->renderPatch($comment, $patch_text, false);
84
+
85
+
$patch_html = null;
86
+
$context_html = $this->renderPatch($comment, $patch_html, true);
73
87
} else {
74
88
$patch_text = $this->getPatch($hunk_parser, $comment, false);
75
89
$context_text = $this->renderPatch($comment, $patch_text, false);
···
374
388
$is_html) {
375
389
376
390
if ($is_html) {
377
-
$patch = $this->renderCodeBlock($patch);
391
+
if ($patch !== null) {
392
+
$patch = $this->renderCodeBlock($patch);
393
+
}
378
394
}
379
395
380
396
$header = $this->renderHeader($comment, $is_html, false);
381
397
382
-
$patch = array(
383
-
$header,
384
-
"\n",
385
-
$patch,
386
-
);
398
+
if ($patch === null) {
399
+
$patch = array(
400
+
$header,
401
+
);
402
+
} else {
403
+
$patch = array(
404
+
$header,
405
+
"\n",
406
+
$patch,
407
+
);
408
+
}
387
409
388
410
if (!$is_html) {
389
411
$patch = implode('', $patch);