Paths

Table of Contentst

Fix passing null to trim() error in DiffusionBrowseTableView render()
ClosedPublic
Actions

Authored by Sten on Jul 4 2023, 15:13.

Details

Reviewers

valerio.bozzolan
avivey

Group Reviewers

O1: Blessed Committers

Maniphest Tasks

T15522: Top level diffusion repository view fails under PHP 8.1 with passing null to trim()

Commits

rP8af16246920c: Fix passing null to trim() error in DiffusionBrowseTableView render()

Summary

When viewing the top level of a diffusion repository, $request->getPath() returns null, which then causes trim() to throw an error.

Fix is to update DiffusionRequest getPath() such that it returns '' instead of null, thus fixing it's return type.

Fixes T15522

Test Plan

Go to https://my.phorge.site/source/myrepo

Diff Detail

Repository

rP Phorge

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Sten created this revision.Jul 4 2023, 15:13

Owners added a reviewer: O1: Blessed Committers.Jul 4 2023, 15:13

Herald added subscribers: Cigaryno, Matthew, valerio.bozzolan and 2 others. · View Herald TranscriptJul 4 2023, 15:13

Sten requested review of this revision.Jul 4 2023, 15:13

Harbormaster completed remote builds in B633: Diff 1066.Jul 4 2023, 15:13

Remove earlier changes to DiffusionBrowseTableView

Harbormaster completed remote builds in B634: Diff 1067.Jul 4 2023, 15:14

valerio.bozzolan added inline comments.Jul 5 2023, 13:11

src/applications/diffusion/request/DiffusionRequest.php
12	Note that, if we are sure that the default should never be NULL, we can just assume this default ↑ But take this tip with small tweezers.

Sten added inline comments.Jul 5 2023, 15:19

src/applications/diffusion/request/DiffusionRequest.php
12	Great minds think alike - this was the first thing I tried, and we still got the error. Seems that $path is being set to null elsewhere.

valerio.bozzolan added inline comments.Jul 5 2023, 15:20

src/applications/diffusion/request/DiffusionRequest.php
12	.__.
257	Maybe you can like this function: `coalesce()` Example use: return coalesce($this->path, '');

Soft +1 but we need some other opinions here

coalesce - I like that. It's going to see some use!

Harbormaster completed remote builds in B648: Diff 1085.Jul 5 2023, 15:44

Sten marked an inline comment as done.Jul 5 2023, 15:45

avivey accepted this revision.Jul 17 2023, 06:02

This revision is now accepted and ready to land.Jul 17 2023, 06:02

Closed by commit rP8af16246920c: Fix passing null to trim() error in DiffusionBrowseTableView render(). · Explain WhyJul 17 2023, 10:05

This revision was automatically updated to reflect the committed changes.

Sten added a commit: rP8af16246920c: Fix passing null to trim() error in DiffusionBrowseTableView render().

Revision Contents
Changeset List

Path

Size

Packages

src/

applications/

diffusion/

request/

DiffusionRequest.php

2 lines

O1: Blessed Committers

Diff 1130

View Options

src/applications/diffusion/request/DiffusionRequest.php

Fix passing null to trim() error in DiffusionBrowseTableView render()ClosedPublicActions