Fixes T11547. I think this mostly gets about addressing @epriestley's comments in D16465 and stores each paste's line count in its snippet so that we can display the actual number of lines in the paste rather than '5 Lines'. Let me know if this is on the right track!
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers - Maniphest Tasks
- T11547: Paste query results do not display actual length of listed Pastes (but always "5 lines" if Paste is longer)
- Commits
- rPbee043b16362: Display paste line count alongside snippets
Open /paste and see that each paste's actual line count is reported.
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- contentLineCount
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 15392 Build 20274: Run Core Tests Build 20273: arc lint + arc unit
Event Timeline
Comment Actions
This looks mostly right to me -- one inline about a weird cast that I don't think we need? We probably don't need it on the other properties either, but there might be some subtle null vs empty string issue there. Here, count(...) only ever returns integers.
Also caught one real bug with count(...), I believe.
| src/applications/paste/query/PhabricatorPasteQuery.php | ||
|---|---|---|
| 331 | Does removing this (string) cast break anything? I'd expect us to be OK without it, and the line count is probably usually an integer. | |
| 365 | I think this never works, since you're counting $line_count, and count($any_integer) is 1, because PHP has ultimate god-tier language design. I believe this should just be if ($line_count > 5), not if (count($line_count) > 5). You can trigger the wrong behavior by doing this, I think:
| |
Comment Actions
Looks good to me, thanks!
I added you to Blessed Committers so you should be able to land this yourself. That project description has some instructions.
You're also now a member of Community, granting you sweeping janitorial powers on this install.