Page MenuHomePhabricator

Addressing some PHP 8 incompatibilities - Remarkup
ClosedPublic

Authored by cspeckmim on May 25 2023, 12:07 AM.
Tags
None
Referenced Files
F14113940: D21866.id52149.diff
Thu, Nov 28, 6:49 AM
F14111809: D21866.diff
Wed, Nov 27, 11:06 PM
Unknown Object (File)
Tue, Nov 26, 2:48 AM
Unknown Object (File)
Sun, Nov 24, 7:54 PM
Unknown Object (File)
Sun, Nov 24, 10:56 AM
Unknown Object (File)
Sat, Nov 23, 6:41 AM
Unknown Object (File)
Wed, Nov 6, 12:59 AM
Unknown Object (File)
Sat, Nov 2, 4:03 PM
Subscribers

Details

Summary

Updating compatibility for PHP 8.2 for remarkup-related functionality.

This also resolves an issue introduced by https://secure.phabricator.com/D21860. Of all the flags when opening the zip the one I chose was not introduced in 5.2 but in 7.4

Refs T13588

Test Plan

I rendered comments using figlet, cowsay, images, object references.

Diff Detail

Repository
rP Phabricator
Branch
cspeck-php8-remarkup
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 25813
Build 35641: arc lint + arc unit

Event Timeline

cspeckmim held this revision as a draft.
cspeckmim added inline comments.
.editorconfig
13–33

I was seeing errors in my editor about indent_style and indent_size needing valid values. The editorconfig web page says

For any property, a value of "unset" is to remove the effect of that property, even if it has been set before.

which I'm assuming is the desired behavior

cspeckmim retitled this revision from Addressing some PHP 8 incompatibilities - Remarpkup to Addressing some PHP 8 incompatibilities - Remarkup.May 26 2023, 9:03 PM
epriestley added inline comments.
src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php
236

This loop is wonky (see the comment above referencing T13487) and the driving report for that task was some pathological nonsense, but probably-prefer this simpler loop body:

if (isset($text[$ii])) {
  $lines .= $text[$ii];
}

The advantage of this approach is that isset() is the same logical test (array key exists, and value is not null), but it's implemented as a PHP language keyword (isset) instead of a function, which is very very slightly faster than a function call.

In D20968, avoiding array_slice() + implode() gave us a 3x speedup on the highly secret pathological input, so it seems worth retaining this since the isset() code is a bit simpler anyway.

This revision is now accepted and ready to land.May 27 2023, 12:26 AM
cspeckmim added inline comments.
src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php
236

will fix!

This revision is now accepted and ready to land.May 28 2023, 10:50 PM