Page MenuHomePhabricator

Initial support for comments/append-edits in EditEngine
ClosedPublic

Authored by epriestley on Dec 2 2015, 7:17 PM.
Tags
None
Referenced Files
F18086263: D14637.id35413.diff
Tue, Aug 5, 8:37 PM
F18056430: D14637.id35444.diff
Mon, Aug 4, 8:31 AM
F17949697: D14637.id.diff
Thu, Jul 31, 11:46 PM
F17948433: D14637.id35444.diff
Thu, Jul 31, 9:06 PM
F17936601: D14637.id35413.diff
Thu, Jul 31, 12:27 AM
F17923402: D14637.diff
Wed, Jul 30, 8:10 AM
F17919984: D14637.id.diff
Wed, Jul 30, 3:08 AM
F17854086: D14637.id35444.diff
Sun, Jul 27, 2:38 PM
Subscribers
None

Details

Summary

Ref T9132. This just replaces the "Add Comment" form in Paste with a generic flow in EditEngine.

No actual field-awareness or action stacking or anything quite yet, but that will come in a bit. This mildly regresses drafts (which don't seem like a big deal for Pastes). I'll hook those up again in the next diff, but I want to build them in a better way that will work with multiple actions in a generic way, and solve T5031.

Big practical advantage here is that applications don't need copy/pasted preview controllers.

Test Plan
  • Saw previews.
  • Added comments.

Diff Detail

Repository
rP Phabricator
Branch
ee29
Lint
Lint Errors
SeverityLocationCodeMessage
Errorsrc/applications/base/PhabricatorApplication.php:300XHP86Unsafe Usage of Dynamic String
Errorsrc/applications/base/PhabricatorApplication.php:302XHP86Unsafe Usage of Dynamic String
Advicesrc/applications/transactions/editengine/PhabricatorEditEngine.php:882XHP16TODO Comment
Unit
Tests Passed
Build Status
Buildable 9224
Build 10921: Run Core Tests
Build 10920: arc lint + arc unit

Event Timeline

epriestley retitled this revision from to Initial support for comments/append-edits in EditEngine.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: chad.
chad edited edge metadata.

Does this work for documents as well? I think Phame is our only commentable Document.

This revision is now accepted and ready to land.Dec 2 2015, 7:42 PM

Not yet, but it should by the time I get there.

This revision was automatically updated to reflect the committed changes.