Page MenuHomePhabricator

Add em and strong shortcuts to phutil_tag
AbandonedPublic

Authored by chad on Dec 8 2015, 9:54 PM.
Tags
None
Referenced Files
F14067898: D14710.id.diff
Tue, Nov 19, 6:03 PM
F14067878: D14710.diff
Tue, Nov 19, 5:49 PM
F14058814: D14710.diff
Sun, Nov 17, 3:00 PM
F14039933: D14710.id35578.diff
Mon, Nov 11, 6:48 AM
F14010311: D14710.id35578.diff
Thu, Oct 31, 7:01 AM
F13995515: D14710.diff
Wed, Oct 23, 1:29 PM
Unknown Object (File)
Oct 10 2024, 3:55 AM
Unknown Object (File)
Sep 10 2024, 4:50 PM
Subscribers

Details

Reviewers
epriestley
Summary

We have hundreds of these calls, maybe shorten them up?

Test Plan

Ask @epriestley if this is ok

Diff Detail

Repository
rPHU libphutil
Branch
phutil_tag
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 9396
Build 11178: Run Core Tests
Build 11177: arc lint + arc unit

Event Timeline

chad retitled this revision from to Add em and strong shortcuts to phutil_tag.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
chad added a subscriber: epriestley.
epriestley edited edge metadata.

What do you think about, say, new PHUIStrongView($content) instead? I don't really like these (or phutil_tag_div()) very much, especially since these take different parameters than phutil_tag_div() does. I worry about a future world where we have like 10 of these and they all behave very slightly differently and the benefit is that we saved like 800 bytes of code in the whole codebase.

That is, if you know what phutil_tag() does, it's totally obvious what phutil_tag('em', array(), $content) does, even if it's a little redundant. Then let's say you learn phutil_tag_em($content), and it's okay because it's very slightly shorter. Then you're like "oh I can shorten div too" so you write phutil_tag_div($content), which breaks because the first parameter to phutil_tag_div() is not $content but $class.

I don't feel too strongly about this, though. See also D7545#48975.

This revision is now accepted and ready to land.Dec 8 2015, 10:02 PM

true those are reasonable concerns