Width overflow fixed

Remove unecessary comment

I personally prefer option 1, so, removing the duplicate body. So there is more creative space to then add a more useful body in the future.

Indeed, we seem to prefer throwing exceptions after if (!($foo instanceof PhutilSortVector)), or use assert_instances_of() on an array, using gettype() and get_class().

This is becoming quite frustrating in my workplace in Torino. Let's start immersion mode.

Ironically, adding missing variable names to @param lines increases the amount of errors in PHPStan output:

Don't remove some empty line

Accepting but please wait other comments - 2 weeks?

...but don't follow my tip, since the above thing forces to a specific class and it's different than the old and already-proposed behavior

I'm personally OK with this, premising that Phorge never used assert(), as far as I can see.

I agree with @waldyrious and I just want to add that, if somebody wants to add a button (and restore multi-line), here is probably the code:

Note that I have NOT properly tested this locally as I still have issues with my LDAP setup.

Confirming my suspicion on PHP 8.3 (while I'm still struggling to successfully bind locally).
As far as I can tell (well, I'm running into ldap_bind(): Unable to bind to server: Can't contact LDAP server after the patch above while my local LDAP server is running but that's nothing new triggered by the patch above), the patch above seems to work.

Also without that comment in 1942. Feel free to omit. It's self-explaining.

The root cause seems an array(null) here in the base case:

Do what Valerio says :)

Thanks for this prototype but unfortunately this approach has the problem that it's verticalized on Maniphest and it should really not be expanded for Phriction or to any other component that may have a description and may suffer the same thing.

Thanks! While it does not work for all and anything (e.g. top bar search button), it is still an improvement when it comes to consistency between mouseclick and return/enter button, so I approve as I haven't seen any fallout and could not find collisions/interference with other existing functionality. (For the records, there is similar code existing in webroot/rsrc/externals/javelin/core/Event.js calling JX.Stratcom.listen() this way.)

How could existing installation be potentially affected by an added option? (The name should be different, of course, if it includes all users from all projects, for example, "Members of linked projects" vs "Project members".)

Do what avivey analyzed (thanks!) and wrote in D25743#20738

@arp: Sure, changing underlying semantics can solve some problems (and create new ones for existing installations). :)

Both proposals would solve the problem. Proposal A is more general, so it would cover more ground but also require more manual configuration.

@aklapper it would totally work if every linked project's users would get access.

In T15914#19057, @aklapper wrote:

Ctrl-clicking on the button opens the form in a new tab

@BlankEclair: Hmm, at least for the magnifier search button in the top bar search field that doesn't seem the case for me (Firefox, Linux). Do I misunderstand?

Edit: Ah, yes, it works with the "Search" button at the bottom of maniphest/query/advanced/, I see.

Ctrl-clicking on the button opens the form in a new tab

Hard to tell if it checks all and anything that could ever be tested (ICS files can be a looot of phun), on the other hand I cannot come up with any downsides of merging this either as it could always be expanded or adjusted and it passes currently for me, so I'll accept.

Fix a bug, grow another bug 🌈 https://t.me/phabricator/889