Page MenuHomePhabricator

Fix the feed line items for autodetect paste languages
ClosedPublic

Authored by jcox on Aug 30 2016, 6:02 PM.
Tags
None
Referenced Files
F12834465: D16474.id39635.diff
Thu, Mar 28, 2:38 PM
Unknown Object (File)
Wed, Mar 20, 6:41 PM
Unknown Object (File)
Feb 24 2024, 5:13 PM
Unknown Object (File)
Feb 22 2024, 3:04 AM
Unknown Object (File)
Feb 22 2024, 3:04 AM
Unknown Object (File)
Feb 20 2024, 1:35 PM
Unknown Object (File)
Feb 15 2024, 4:48 PM
Unknown Object (File)
Feb 14 2024, 10:55 AM
Tokens
"Dat Boi" token, awarded by epriestley.

Details

Summary

Fixes T11555. Previously changing the extension of a paste wouldn't change the syntax highlight language. Now it does.
Also, feed items involving autodetect weren't rendering in a readable way.

Test Plan

Created a paste named paste.php and let it autodetect language. Then edited the paste to be named paste.rainbow. It should now be highlighted in raiinnboow

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

jcox retitled this revision from to Fix the feed line items for autodetect paste languages.
jcox updated this object.
jcox edited the test plan for this revision. (Show Details)
epriestley added a reviewer: epriestley.

Let's call emptyToAutodetect() something like renderLanguageValue()? Because empty is both an adjective and a verb, I find method names starting with it are often pretty confusing -- the interpretation of "make empty" vs "test for empty" is often unclear (and this is neither, using empty as a noun I guess?). See here for one example:

https://secure.phabricator.com/book/phabcontrib/article/general_coding_standards/#naming-things

The most natural interpretation of emptyToAutodetect() for me is "empty [something out of this object in order] to [perform] autodetect", not "[convert] empty [values] to [the string] 'autodetect'".

empty() is not the best test to use here, because these values are empty():

0
0.0
"0"
""

Although it's unlikely that any of them will appear in this context, it's better to use a more precise test because they do appear in some contexts. Here's a real-world example of this kind of data appearing in a surprising place in the wild, when the user with GitHub username 0 (https://github.com/0) tried to register for an account on this install: rP0e672e2435581844b54bb1f91cdb4bb740a38d97

Better tests are !== null (detects null exactly) or strlen(...) (detects null or empty string).

src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php
18

This should be pht()'d since it's human-facing.

This revision now requires changes to proceed.Aug 30 2016, 6:14 PM
jcox edited edge metadata.

Rename weirdly named function

epriestley edited edge metadata.
This revision is now accepted and ready to land.Aug 30 2016, 6:26 PM
This revision was automatically updated to reflect the committed changes.
jcox marked an inline comment as done.