Paths

Table of Contentst

Differential D19437

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

Authored by epriestley on May 9 2018, 5:56 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Apr 17, 2:56 PM

	Unknown Object (File)
	Tue, Apr 16, 4:22 PM

	Unknown Object (File)
	Tue, Apr 16, 12:12 PM

	Unknown Object (File)
	Tue, Apr 9, 5:56 AM

	Unknown Object (File)
	Wed, Mar 27, 11:41 AM

	Unknown Object (File)
	Wed, Mar 27, 11:39 AM

	Unknown Object (File)
	Wed, Mar 27, 11:39 AM

	Unknown Object (File)
	Wed, Mar 27, 11:39 AM

Subscribers

None

Details

Reviewers

Commits

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

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.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley created this revision.May 9 2018, 5:56 PM

Harbormaster completed remote builds in B20299: Diff 46491.May 9 2018, 5:57 PM

epriestley requested review of this revision.May 9 2018, 5:57 PM

amckinley accepted this revision.May 9 2018, 8:04 PM

amckinley added inline comments.

src/applications/paste/conduit/PasteCreateConduitAPIMethod.php
70	Add an empty string check to `PhabricatorPasteLanguageTransaction->validateTransactions()`?

This revision is now accepted and ready to land.May 9 2018, 8:04 PM

Add an empty string check

I was thinking this would break stuff, but I think I was a little muddled and that it's actually safe. Gimmie a sec..

Validate that language is null or nonempty string.
Edited a paste to autodetect/empty/null, and to a real language like "XML".
Hit the error by manually crafting a Conduit request.

Harbormaster completed remote builds in B20304: Diff 46496.May 9 2018, 8:11 PM

Closed by commit rPd280b24239fc: Fix "arc paste" to stop creating pastes with an empty string ("") as the… (authored by epriestley). · Explain WhyMay 9 2018, 8:23 PM

This revision was automatically updated to reflect the committed changes.

epriestley mentioned this in D19456: Fix Javascript busy loop when trying to delete tokens from an empty tokenizer.Jun 1 2018, 8:08 PM

epriestley mentioned this in rP31ee49b14d93: Fix Javascript busy loop when trying to delete tokens from an empty tokenizer.Jun 1 2018, 9:51 PM

Revision Contents
Changeset List

Path

Size

resources/

celerity/

src/

applications/

paste/

conduit/

PasteCreateConduitAPIMethod.php

9 lines

xaction/

PhabricatorPasteLanguageTransaction.php

16 lines

webroot/

rsrc/

externals/

javelin/

lib/

control/

tokenizer/

6 lines

Diff 46499

resources/celerity/map.php

Loading...

src/applications/paste/conduit/PasteCreateConduitAPIMethod.php

Loading...

src/applications/paste/xaction/PhabricatorPasteLanguageTransaction.php

Loading...

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

Loading...