Page MenuHomePhabricator

Addressing some PHP 8 incompatibilities - Remarkup
ClosedPublic

Authored by cspeckmim on May 25 2023, 12:07 AM.
Tags
None
Referenced Files
F13253219: D21866.diff
Sat, May 25, 2:30 AM
F13232274: D21866.diff
Tue, May 21, 1:10 AM
F13218097: D21866.id52148.diff
Sat, May 18, 10:09 AM
F13217491: D21866.id52158.diff
Sat, May 18, 5:46 AM
F13210673: D21866.diff
Fri, May 17, 5:06 AM
F13206921: D21866.id52152.diff
Wed, May 15, 7:27 PM
F13198377: D21866.diff
Mon, May 13, 6:00 AM
F13196190: D21866.diff
Sun, May 12, 11: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
Lint
Lint Not Applicable
Unit
Tests Not Applicable

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