Page MenuHomePhabricator

Fallback to placeholder if datasource placeholder does not exist
AbandonedPublic

Authored by chad on Jul 19 2017, 2:43 AM.
Tags
None
Referenced Files
F13256158: D18240.diff
Sat, May 25, 9:56 AM
F13250232: D18240.diff
Fri, May 24, 1:26 PM
F13233213: D18240.diff
Tue, May 21, 1:58 AM
F13208373: D18240.diff
Thu, May 16, 2:46 PM
F13207837: D18240.id43870.diff
Thu, May 16, 4:45 AM
F13192767: D18240.diff
Sun, May 12, 9:39 AM
F13175824: D18240.diff
Wed, May 8, 7:46 AM
Unknown Object (File)
Mon, May 6, 2:04 PM
Subscribers

Details

Reviewers
epriestley
Summary

Community Report: https://discourse.phabricator-community.org/t/cannot-display-placeholder-in-src-view-form-control-aphrontformtokenizercontrol-php/

All tokenizers I checked seemed to work OK. I think the Locate File field in Diffusion uses this.

Test Plan

Diffusion, various other placeholders.

Diff Detail

Repository
rP Phabricator
Branch
placeholder (branched from master)
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 17734
Build 23811: Run Core Tests
Build 23810: arc lint + arc unit

Event Timeline

  • don't think I did that right

How do I reproduce the problem this causes?

The code does seem wrong though?

I'll admit I missed reproduction steps on first pass (and didn't ask until after I wrote the diff)

I don't see any callsites, so I believe this doesn't cause any reproducible bad behavior -- i.e., this is support for third-party development, not a bug fix. If there are no callsites, we should just remove setPlaceholder() and the placeholder property if you want to clean this up.

(Without blaming it, my guess is that this method predates the modularization of TypeaheadDatasource into a separate subclass and has been obsolete since then.)

Not the worst thing I've wasted my time on today.

it's a nice diff though. I used a getter.