Page MenuHomePhabricator

Modularize parser for "Closes task X as Y"
ClosedPublic

Authored by epriestley on Feb 17 2014, 7:01 PM.
Tags
None
Referenced Files
F14005907: D8261.id19659.diff
Sun, Oct 27, 9:41 PM
F13989418: D8261.id19659.diff
Mon, Oct 21, 8:19 PM
F13977885: D8261.diff
Fri, Oct 18, 8:29 PM
F13973404: D8261.id.diff
Fri, Oct 18, 12:47 AM
F13966781: D8261.id19649.diff
Wed, Oct 16, 9:37 AM
Unknown Object (File)
Fri, Oct 11, 5:14 PM
Unknown Object (File)
Fri, Oct 11, 4:04 PM
Unknown Object (File)
Fri, Oct 11, 4:04 PM
Subscribers

Details

Reviewers
btrahan
Maniphest Tasks
Restricted Maniphest Task
T1812: Allow for arbitrary values for task status
Restricted Maniphest Task
Commits
Restricted Diffusion Commit
rPd016cac91513: Modularize parser for "Closes task X as Y"
Summary

Ref T3886. Ref T3872. Ref T1812. We have several parsers which look for textual references to other objects, like:

Closes Tx.
Depends on Dy.
Reverts Dz.

Currently, these are pretty hard coded, don't get all the edge cases right, and don't generalize well. They're also implemented in the middle of Differential's field code. So I want to:

  • Share more code so that, e.g., "Tx, Ty" always works (only some rules support it right now);
  • fix bugs in the parser, like T3872;
  • make this a modular, extensible process which runs against custom fields, not a builtin part of fields;
  • make the internals more flexible to accommodate custom stuff like T1812.

This implements the "Verbs optional-noun Object, Optional Other Objects optional-as-something." grammar in a general way so subclasses can just plug in their keywords. Runtime code doesn't touch this yet.

Test Plan

Ran unit tests.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

I'm intentionally ignoring internationalization here for now. This positions us better for it than we were positioned before, but if we pht() these strings it prevents other languages from having more or fewer trigger phrases. We could do something like this:

$phrases = pht('a,b,c,d');
return explode(',', $phrases);

However, this would probably be pretty non-obvious to translators. We could do this:

$phrases = pht('Enter a comma-separated list of prefix phrases for marking a dependency (English = a,b,c,d).');

...and then translate that to "a,b,c,d" in English, but I'd rather wait until someone actually wants to translate this and/or we get more similar cases before doing that, since it feels a bit rough.