Page MenuHomePhorge

Sten (Steve Campbell)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Thursday

  • Clear sailing ahead.

User Details

User Since
Aug 8 2022, 10:07 (102 w, 16 h)
Availability
Available

Recent Activity

May 8 2024

Sten added a comment to T15804: Imbalanced AphrontWriteGuard: more beginUnguardedWrites() calls than endUnguardedWrites() calls.

Had this issue occur elsewhere, so put a general 'fix' for it:

May 8 2024, 13:10

Apr 30 2024

Sten added a comment to T15804: Imbalanced AphrontWriteGuard: more beginUnguardedWrites() calls than endUnguardedWrites() calls.

To find it, I needed to put in loads of debugging. It seems to be an issue with PHP deferring the execution of the object destructor until after the Imbalanced AphrontWriteGuard check, even though the object was unset prior to the check.

Apr 30 2024, 15:07
Sten updated the task description for T15804: Imbalanced AphrontWriteGuard: more beginUnguardedWrites() calls than endUnguardedWrites() calls.
Apr 30 2024, 10:19
Sten created T15804: Imbalanced AphrontWriteGuard: more beginUnguardedWrites() calls than endUnguardedWrites() calls.
Apr 30 2024, 10:18

Apr 8 2024

Sten added a comment to T15751: Make Libphutil and Linters support modern PHP syntax.

I wonder if xhpast can be ditched. Eg:
For linters, use something off-the-shelf such as phpcs (https://github.com/squizlabs/PHP_CodeSniffer)
For diviner parsing of PHP code, I look at the class documentation (eg https://we.phorge.it/book/libphutil/class/ArcanistConduitCallFuture/) and question as to whether this provides anyone with any benefit.

Apr 8 2024, 15:17 · Phorge Development Tools
Sten updated the task description for T15751: Make Libphutil and Linters support modern PHP syntax.
Apr 8 2024, 12:08 · Phorge Development Tools
Sten added a comment to T15047: Officially raise minimum required PHP version to 7.2.
In T15047#1308, @speck wrote:
Apr 8 2024, 10:26 · Phorge

Mar 19 2024

Sten added a comment to T15759: MySQL edge table error log: 'INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe'.

Sorry, I'm not explaining it very well. For the database engine to use index (col1, col2, col3), col1 must be specified somewhere in the WHERE clause, but it's precise location is irrelevant - the database engine will work it out. As to why we need both indexes, there's a good explanation on https://use-the-index-luke.com/sql/where-clause/the-equals-operator/concatenated-keys

Mar 19 2024, 22:35
Sten added a comment to T15759: MySQL edge table error log: 'INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe'.

@avivey - the order of the keys is irrelevant when it comes to determining uniqueness, but extremely important in other respects. In order for an index to be used when selecting rows, the database must be provided with values for the index fields, in the order specified. Ie you always need to specify a value for the first field, and can get away without successive ones.

Mar 19 2024, 13:50

Mar 18 2024

Sten added a comment to T15759: MySQL edge table error log: 'INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe'.

@valerio.bozzolan agreed - I'm not proposing to remove the (dst, type, src) index, just the uniqueness constraint on that index. Sorry for not making that clear.

Mar 18 2024, 11:19
Sten created T15759: MySQL edge table error log: 'INSERT... ON DUPLICATE KEY UPDATE on a table with more than one UNIQUE KEY is unsafe'.
Mar 18 2024, 10:49

Mar 15 2024

Sten claimed T15757: Arcanist Test Result Parser Updates.
Mar 15 2024, 16:14 · Arcanist
Sten created T15757: Arcanist Test Result Parser Updates.
Mar 15 2024, 16:14 · Arcanist
Sten closed T15667: Update PhpunitTestEngine to not call phpunit with --log-json option as Resolved by committing rARC7c5e607e9752: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use….
Mar 15 2024, 16:09 · Arcanist
Sten closed D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Mar 15 2024, 16:09
Sten committed rARC7c5e607e9752: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use….
Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use…
Mar 15 2024, 16:09

Mar 12 2024

Sten added a comment to D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..

Possible further enhancements, which I am loathe to do when trying to fix an existing bug, so perhaps as a future diff:

Mar 12 2024, 18:39
Sten updated the diff for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..

As per @avivey, update ArcanistPhpunitTestResultParser.php to call ArcanistXUnitTestResultParser and remove it's own XUnit parsing code.

Mar 12 2024, 18:30

Mar 10 2024

Sten added a comment to D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..

@speck , @valerio.bozzolan
Sorry, yes, it's updating the test engine to parse phpunit's junit output, rather than the json output which no longer exists.

Mar 10 2024, 19:41

Dec 1 2023

Sten added a comment to D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..

Testing, using https://github.com/campbsb/example-phorge-php-project.git and breaking it so as to cause a test failure, we get:

Dec 1 2023, 16:31
Sten updated the diff for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
  • Add LIBXML_NONET option
  • Remove $last_test_finished and the !last_test_finished block, as that is always true
Dec 1 2023, 16:25

Nov 27 2023

Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

When a phriction markup page is generated , its synchronous, when a diffusion markup page is generated it's asynchronous.
This is going to take a bit of thought.

Nov 27 2023, 16:05

Nov 21 2023

Sten closed T15641: Creating or editing a project under PHP 8.1 causes strlen() null error with custom integer fields, a subtask of T15064: Make Phorge compatible with PHP 8.1/8.2/8.3, as Wontfix.
Nov 21 2023, 16:09 · PHP 8 support
Sten closed T15641: Creating or editing a project under PHP 8.1 causes strlen() null error with custom integer fields as Wontfix.

Can no longer replicate the fault.

Nov 21 2023, 16:09 · PHP 8 support
Sten abandoned D25439: Fix strlen(null) errors for projects with integer fields under PHP 8.1.

I can no longer replicate the fault, so am going to scrap this one.

Nov 21 2023, 16:09

Nov 20 2023

valerio.bozzolan awarded T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup) a Love token.
Nov 20 2023, 09:15 · Feature Requests

Nov 15 2023

Sten added a comment to T15501: Voluntary Usage Survey App.

One of my favourite updates in my workplace was to store counts of feature usage per month in a table. Move forward 3 years, and whenever we need to update some code in a feature, we check for it in the table, and if it's not been used in 3 years we just scrap the feature.

Nov 15 2023, 18:14 · Discussion Needed
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Skip lint (javelinsymbols)

Nov 15 2023, 17:49
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Great suggestions

  • Implemented celerity_generate_unique_node_id()
  • Implemented .collapsible-content > ul.remarkup-list CSS

Loved the comment, as otherwise I wouldn't have understood what was going on in the CSS. I've reworded it - hope that's OK.

Nov 15 2023, 17:49
Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

I am quite happy with this now. Spent way too long on it, so I hope you like it too!

Nov 15 2023, 15:25
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
  • Refactored the PHP code to build the collapsible button & div into buildCollapsibleIndex()
  • Define the javelin call config first, then refer to it - saves defining the variables independently!
  • Create an icon field in the button, allowing for open and close icons to be set
  • Make both the icon and the text optional in the collapsible javascript - if you just want an icon, fine. Just text, also fine. Neither - sure!
Nov 15 2023, 15:23
Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

The problem there is that by requiring collapsiblejx to have knowledge of the 'collapsible-toggler-label', we introduce tight coupling to it's caller. (see how adding 'jx' on to the end of collapsiblejx has made it uniquely identifiable?)
A better solution would be to have the collapsiblejx config contain 'btn_open_icon' and 'btn_close_icon' icons. I'll have a go...

Nov 15 2023, 09:52
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
  • move duplicated button text into a variable
  • improve formatting
Nov 15 2023, 08:24
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
  • Renamed button/div IDs to remove spaces
  • Renamed diffusion-collapsible-container to collapsiblejx
  • Added margin-top: 10px; in the collapsible content - this overrides the higher up margin-top: 0px
  • Moved webroot/rsrc/js/application/diffusion/collapsible-container.js to webroot/rsrc/js/core/behavior-collapsiblejx.js
  • Renamed the JX from diffusion-collapsible-container to collapsiblejx - the single word collapsible was a bit too common if you 'git grep' for it.
Nov 15 2023, 08:14

Nov 14 2023

Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Renamed butt to btn as per PR. Much more common abbrieviation!

Nov 14 2023, 20:37
valerio.bozzolan awarded D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup a Love token.
Nov 14 2023, 16:40
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Happy to take your suggestions with a couple of changes

  • Remove index from the config labels - we've made collapsible-container.js generic, and not just for an index.
  • The reason for the if/else block was to enable long variable/attribute names so as to have self-documenting code. I've done away with it, but we do need a comment instead so as to show why we associate the 'push this to close' text with the open state and vice-versa.
Nov 14 2023, 15:33
Sten closed D25436: Update PhutilCowsay.php to work for small cows.
Nov 14 2023, 15:05
Sten committed rARC5bc53cfe53d0: Update PhutilCowsay.php to work for small cows.
Update PhutilCowsay.php to work for small cows
Nov 14 2023, 15:05
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

As an aside, what do you think of https://github.com/campbsb/example-phorge-php-project - should we put a selection of example projects on https://we.phorge.it, either as individual repos, or one repo containing a bunch of examples, or somewhere in the arcanist repo ?

Nov 14 2023, 11:17 · Arcanist
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

More complete test case added to D25472 which demonstrates the following error:

Nov 14 2023, 11:10 · Arcanist
Sten updated the test plan for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Nov 14 2023, 11:06
Sten updated the test plan for D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Nov 14 2023, 09:57
Sten requested review of D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Nov 14 2023, 09:57
Sten added a revision to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option: D25472: Update PhpunitTestEngine.php and ArcanistPhpunitTestResultParser.php to use junit output rather than json..
Nov 14 2023, 09:57 · Arcanist
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

I guess the documentation needs an update on page https://we.phorge.it/book/phorge/article/arcanist_lint_unit/ and/or https://we.phorge.it/book/phorge/article/arcanist_new_project/

Nov 14 2023, 09:28 · Arcanist
Sten added a comment to T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.

6 year old in-your-face unreported bugs like these make me wonder how many people use Phabricator/Phorge :-(

Nov 14 2023, 09:22 · Arcanist
Sten claimed T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.
Nov 14 2023, 07:40 · Arcanist
Sten created T15667: Update PhpunitTestEngine to not call phpunit with --log-json option.
Nov 14 2023, 07:40 · Arcanist

Nov 12 2023

Sten added a comment to D25436: Update PhutilCowsay.php to work for small cows.

Any problems with this fix?

Nov 12 2023, 11:13
Sten added a comment to D25439: Fix strlen(null) errors for projects with integer fields under PHP 8.1.

Is switching to using phutil_nonempty_scalar a blocker?
I don't think it should be because otherwise, we could have just done a bulk change across the whole codebase converting

Nov 12 2023, 11:12
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
  • Javascript updated to make collapsible containers more generic, and not specific to indices
  • Removed unused 'active' as per review
  • Moved setting the display of the container from Javascript to CSS as per review
  • Added i18n support for the button text (no need for a TODO!)
  • Extended variable/attribute names to make the code a bit more self-documenting
Nov 12 2023, 11:01

Nov 7 2023

Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Remove "'hidden' => true," line from javelin config - no longer required.

Nov 7 2023, 23:03
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Update the button such that it toggles between 'Open Index' and 'Close Index'

Nov 7 2023, 22:59
Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Non-blocking stuff:

  1. Maybe we should also evaluate this?
    Arrow indicating a proposal UX change in a Phorge README.png (168×1 px, 14 KB)
Nov 7 2023, 22:47
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Rebuild celerity map

Nov 7 2023, 22:43
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Skipping lint because I cannot get javelinsymbols from https://github.com/phacility/javelin/tree/master to compile under OSX
Actually, it's the out-of-support 13 year old libfbjs (from https://github.com/facebookarchive/libfbjs) from it which won't compile.
I recommend javelin linting is dropped, and replaced with a more modern javascript linter.

Nov 7 2023, 22:42
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Move javascript into a Javelin event.
Correct mistake in Javelin documentation.

Nov 7 2023, 21:46
Sten added inline comments to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 7 2023, 21:46

Nov 6 2023

Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

I like the general idea but I don't know how to improve it graphically. I mean, this is the current rendering for a simple README of two sections:

I don't think any document with a header to text line ratio of 2:3 is going to look good without customised formatting.

Nov 6 2023, 21:50
Sten added a comment to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

The Javascript is inline for now, and we can look at putting it somewhere better if this diff is worth pursuing. Suggestions of where to put it welcome!

Nov 6 2023, 21:47
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Replace method_exists with instance of, as per review

Nov 6 2023, 21:36
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Replacing 'new PhutilSafeHTML' with 'phutil_implode_html' as per review. Much nicer!

Nov 6 2023, 08:04

Nov 5 2023

Sten requested review of D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Looks OK from XSS perspective - protection already in place. Ready for review!

Nov 5 2023, 17:04
Sten planned changes to D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Need to put in some XSS protection.

Nov 5 2023, 16:16
Sten updated the diff for D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.

Restore fixed interpreter-test.txt

Nov 5 2023, 16:06
Sten requested review of D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 5 2023, 15:57
Sten added a revision to T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup): D25457: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 5 2023, 15:57 · Feature Requests
Sten abandoned D25456: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 5 2023, 15:55
Sten requested review of D25456: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 5 2023, 15:54
Sten added a revision to T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup): D25456: Update diffusion browse to generate Table of Contents when viewed as remarkup.
Nov 5 2023, 15:54 · Feature Requests
Sten claimed T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup).
Nov 5 2023, 15:13 · Feature Requests

Nov 3 2023

Sten added a comment to T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup).

An example of the TOC in action is in the diviner pages.

Nov 3 2023, 11:45 · Feature Requests
Sten created T15660: Generate a Table of Content for Diffusion browse as Remarkup (?as=remarkup).
Nov 3 2023, 10:30 · Feature Requests

Oct 25 2023

Sten closed T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated, a subtask of T15064: Make Phorge compatible with PHP 8.1/8.2/8.3, as Resolved.
Oct 25 2023, 10:03 · PHP 8 support
Sten closed T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated as Resolved by committing rP318d7a61feab: Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error.
Oct 25 2023, 10:03 · PHP 8 support
Sten closed D25449: Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error.
Oct 25 2023, 10:03
Sten committed rP318d7a61feab: Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error.
Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error
Oct 25 2023, 10:03
Sten requested review of D25449: Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error.
Oct 25 2023, 08:43
Sten added a revision to T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated: D25449: Fix PhabricatorAuthCSRFEngine.php strncmp(null) PHP 8.1 error.
Oct 25 2023, 08:43 · PHP 8 support
Sten updated the task description for T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated.
Oct 25 2023, 08:22 · PHP 8 support
Sten claimed T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated.
Oct 25 2023, 08:21 · PHP 8 support
Sten created T15654: PhabricatorAuthCSRFEngine.php:50 strncmp(): Passing null to parameter #1 ($string1) of type string is deprecated.
Oct 25 2023, 08:21 · PHP 8 support
Sten updated the task description for T15641: Creating or editing a project under PHP 8.1 causes strlen() null error with custom integer fields.
Oct 25 2023, 08:05 · PHP 8 support

Oct 12 2023

Sten updated the diff for D25436: Update PhutilCowsay.php to work for small cows.

smallcow test case added.

Oct 12 2023, 16:12

Oct 11 2023

Sten updated the diff for D25436: Update PhutilCowsay.php to work for small cows.

Replace $m with $matches, as per review.

Oct 11 2023, 07:31
valerio.bozzolan awarded D25436: Update PhutilCowsay.php to work for small cows a Love token.
Oct 11 2023, 05:56

Sep 28 2023

Sten closed T15647: 'arc lint' fails for composer.json with RuntimeException: Undefined array key "hash" as Resolved by committing rARCba42b63704b2: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Sep 28 2023, 08:12
Sten closed D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Sep 28 2023, 08:12
Sten committed rARCba42b63704b2: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Update ArcanistComposerLinter.php to check content-hash instead of hash
Sep 28 2023, 08:12

Sep 27 2023

Sten added a comment to T15647: 'arc lint' fails for composer.json with RuntimeException: Undefined array key "hash".

This issue has been a pain for years. Never investigated it before...

Sep 27 2023, 09:42
Sten updated the diff for D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.

Fix unit test

Sep 27 2023, 09:40
Sten updated the diff for D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.

Lint & fix xml unit test

Sep 27 2023, 09:33
Sten planned changes to D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Sep 27 2023, 09:12
Sten requested review of D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Sep 27 2023, 09:11
Sten added a revision to T15647: 'arc lint' fails for composer.json with RuntimeException: Undefined array key "hash": D25442: Update ArcanistComposerLinter.php to check content-hash instead of hash.
Sep 27 2023, 09:11
Sten claimed T15647: 'arc lint' fails for composer.json with RuntimeException: Undefined array key "hash".
Sep 27 2023, 08:52
Sten created T15647: 'arc lint' fails for composer.json with RuntimeException: Undefined array key "hash".
Sep 27 2023, 08:52

Sep 20 2023

Sten added inline comments to D25439: Fix strlen(null) errors for projects with integer fields under PHP 8.1.
Sep 20 2023, 22:55
Sten updated the test plan for D25439: Fix strlen(null) errors for projects with integer fields under PHP 8.1.
Sep 20 2023, 20:05