This seems fine, though I wonder if we should introduce a phutil_empty_string() or phutil_is_empty_string function to avoid the double-negative logic. I think this reads more easily:

if (phutil_empty_string($from)) {
  throw new Exception()
}

than this:

if (!phutil_nonempty_string($from)) {
  throw new Exception()
}

This revision is now accepted and ready to land.Dec 2 2023, 19:10

In the future it may be nice to also have a getArgStr() that always return a string, never NULL. So, we can just if ( $to === '' )

valerio.bozzolan accepted this revision.Dec 5 2023, 00:19

@jeanguyomarch: Hi, would you like to arc land your accepted patch?

Closed by commit rPa7fb04f96c92: fix PHP 8 "strlen(null)" when moving paths with missing options. · Explain WhyFeb 12 2024, 10:05

This revision was automatically updated to reflect the committed changes.

valerio.bozzolan added a commit: rPa7fb04f96c92: fix PHP 8 "strlen(null)" when moving paths with missing options.

Revision Contents
Changeset List

Path

Size

Packages

src/

applications/

repository/

management/

PhabricatorRepositoryManagementMovePathsWorkflow.php

4 lines

O1: Blessed Committers

Diff 1691

View Options

src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php

fix PHP 8 "strlen(null)" when moving paths with missing optionsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1691

src/applications/repository/management/PhabricatorRepositoryManagementMovePathsWorkflow.php

fix PHP 8 "strlen(null)" when moving paths with missing options
ClosedPublic
Actions

Revision Contents
Changeset List