Fix "arc paste" to stop creating pastes with an empty string ("") as the "language"

Summary:
See PHI652. When you `echo x | arc paste` today, you end up with a Paste object that has the empty string as its "language".

This is normally not valid. Pastes where the language should be autodetected should have the value `null`, not the empty string.

This behavior likely changed when `paste.create` got rewritten in terms of `paste.edit`. Adjust the implementation so it only adds the LANGUAGE transaction if there's an actual language.

Also, fix an issue where you can't use the "delete" key to delete tokens with the empty string as their value.

Test Plan:
- Created a paste with `echo x | arc paste`, got a paste in autodetect mode instead of with a bogus language value.
- Created a paste with `echo x | arc paste --lang rainbow`, got a rainbow paste.
- Deleted an empty string token with the keyboard.
- Deleted normal tokens with the keyboard.
- Edited subscribers/etc normally with the keyboard and mouse to make sure I didn't ruin anything.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D19437

epriestley 8 years ago d280b242 26c0db8d

+36 -13

4 changed files

expand all

resources

celerity

map.php

src

applications

paste

conduit

PasteCreateConduitAPIMethod.php

xaction

PhabricatorPasteLanguageTransaction.php

webroot

rsrc

externals

javelin

lib

control

tokenizer

Tokenizer.js

+9 -9

resources/celerity/map.php

··· 10 10 'conpherence.pkg.css' => 'e68cf1fa', 11 11 'conpherence.pkg.js' => '15191c65', 12 12 'core.pkg.css' => '8be474cc', 13 - 'core.pkg.js' => 'e1f0f7bd', 13 + 'core.pkg.js' => 'e452721e', 14 14 'differential.pkg.css' => '06dc617c', 15 15 'differential.pkg.js' => 'c2ca903a', 16 16 'diffusion.pkg.css' => 'a2d17c7d', ··· 259 259 'rsrc/externals/javelin/lib/__tests__/URI.js' => '1e45fda9', 260 260 'rsrc/externals/javelin/lib/__tests__/behavior.js' => '1ea62783', 261 261 'rsrc/externals/javelin/lib/behavior.js' => '61cbc29a', 262 - 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => '8d3bc1b2', 262 + 'rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js' => 'dfaf006b', 263 263 'rsrc/externals/javelin/lib/control/typeahead/Typeahead.js' => '70baed2f', 264 264 'rsrc/externals/javelin/lib/control/typeahead/normalizer/TypeaheadNormalizer.js' => '185bbd53', 265 265 'rsrc/externals/javelin/lib/control/typeahead/source/TypeaheadCompositeSource.js' => '503e17fd', ··· 710 710 'javelin-scrollbar' => '9065f639', 711 711 'javelin-sound' => '949c0fe5', 712 712 'javelin-stratcom' => '327f418a', 713 - 'javelin-tokenizer' => '8d3bc1b2', 713 + 'javelin-tokenizer' => 'dfaf006b', 714 714 'javelin-typeahead' => '70baed2f', 715 715 'javelin-typeahead-composite-source' => '503e17fd', 716 716 'javelin-typeahead-normalizer' => '185bbd53', ··· 1573 1573 'javelin-stratcom', 1574 1574 'javelin-behavior', 1575 1575 ), 1576 - '8d3bc1b2' => array( 1577 - 'javelin-dom', 1578 - 'javelin-util', 1579 - 'javelin-stratcom', 1580 - 'javelin-install', 1581 - ), 1582 1576 '8d4a8c72' => array( 1583 1577 'javelin-install', 1584 1578 'javelin-dom', ··· 2024 2018 'javelin-dom', 2025 2019 'phuix-icon-view', 2026 2020 'phabricator-prefab', 2021 + ), 2022 + 'dfaf006b' => array( 2023 + 'javelin-dom', 2024 + 'javelin-util', 2025 + 'javelin-stratcom', 2026 + 'javelin-install', 2027 2027 ), 2028 2028 'e1d25dfb' => array( 2029 2029 'javelin-behavior',

+6 -3

src/applications/paste/conduit/PasteCreateConduitAPIMethod.php

··· 64 64 ->setTransactionType(PhabricatorPasteTitleTransaction::TRANSACTIONTYPE) 65 65 ->setNewValue($title); 66 66 67 - $xactions[] = id(new PhabricatorPasteTransaction()) 68 - ->setTransactionType(PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE) 69 - ->setNewValue($language); 67 + if (strlen($language)) { 68 + $xactions[] = id(new PhabricatorPasteTransaction()) 69 + ->setTransactionType( 70 + PhabricatorPasteLanguageTransaction::TRANSACTIONTYPE) 71 + ->setNewValue($language); 72 + } 70 73 71 74 $editor = id(new PhabricatorPasteEditor()) 72 75 ->setActor($viewer)

+16

src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php

··· 38 38 $this->renderLanguageValue($this->getNewValue())); 39 39 } 40 40 41 + public function validateTransactions($object, array $xactions) { 42 + $errors = array(); 43 + 44 + foreach ($xactions as $xaction) { 45 + $new = $xaction->getNewValue(); 46 + 47 + if ($new !== null && !strlen($new)) { 48 + $errors[] = $this->newInvalidError( 49 + pht('Paste language must be null or a nonempty string.'), 50 + $xaction); 51 + } 52 + } 53 + 54 + return $errors; 55 + } 56 + 41 57 }

+5 -1

webroot/rsrc/externals/javelin/lib/control/tokenizer/Tokenizer.js

··· 395 395 break; 396 396 case 'delete': 397 397 if (!this._focus.value.length) { 398 + // In unusual cases, it's possible for us to end up with a token 399 + // that has the empty string ("") as a value. Support removal of 400 + // this unusual token. 401 + 398 402 var tok; 399 - while ((tok = this._tokens.pop())) { 403 + while ((tok = this._tokens.pop()) !== null) { 400 404 if (this._remove(tok, true)) { 401 405 break; 402 406 }

Configure Feed

Configure Feed