• Ignores async jshint

First of all, I tested this, and this is super-lovely, really better than I thought, I love this. Thanks.

oh, for this one we can just ignore it. arc only shows these for new lines

In D25536#15422, @avivey wrote:

I'll try to see if there's config for the jshint to allow async.

I'm happy about the JS stuff, and I'm ok with any UI solution here.
I'll try to see if there's config for the jshint to allow async.

Well, now I have tried to satisfy every wish.

• use async/await for copy promise hell
• make the io button to just a type icon
• select entire text when clicking inside the input field
• patch JsShrink to shrink await/async syntax

We can probably just pull the JsShrink fix in for now, and later replace both it and jsxmin (which I'll bet nobody have) with something else that is maintained, like JShrink.

(This Quickfix would help: https://github.com/vrana/JsShrink/pull/6/commits/5b2dd500d25b29ace2fbd76fc95b4518ef22abfc 😆 )

await/async won't go through the shrink process without destroying the entire syntax. And if it will go through this jsxmin binary, who can know? Note for the future: Maybe replace the entire shrink process with a new and used php shrinker like JShrink.

In D25536#15408, @valerio.bozzolan wrote:

(Just to clarify, and sorry if late comment again - I was proposing to avoid to add buttons, and simply auto-select and auto-copy the input text the first time it receives a click. Bonus: with "copied" message feedback - I have untested this sorry)

@avivey worst case is a syntax error because the operators are not known. Even that could be wrapped inside a try/catch.
@valerio.bozzolan could be done with the current behavior but this kind of implicit actions seems not the phorge way...?

(Just to clarify, and sorry if late comment again - I was proposing to avoid to add buttons, and simply auto-select and auto-copy the input text the first time it receives a click. Bonus: with "copied" message feedback - I have untested this sorry)

Our policy in general is "support things way after the go EOL", but with the cost of supporting IE 11 (this kind of hell), I think we can make an exception and throw it under the bus. Worst case, we're talking about an error in the logs and "nothing happening" from the user POV, right?

I've successfully updated our Phabricator instance to Phorge today. After paying very careful attention to everything it went smoothly without any issues. Glad that this project has not died. Phabricator grew on me as well.

In D25536#15395, @avivey wrote:

Yes, I insist on using the absolute required minimum of Promise-style coding.

Should we add FreeBSD to this list? It uses Phorge now (as of June 2023) and has many users.
(Much of their documentation still refers to it as "Phabricator", despite this.)

I agree, disabled users should be hidden in the global search typeahead results.

In T15736#15681, @bekay wrote:

• Get rid of the JX namespace and the the require comments - use import and export like it is done in modern js

Remove unreached return call as pointed out by speck

In D25502#15384, @speck wrote:

Is it possible to check if anything is relying on this not working with negative numbers?

Sorry for my late comment

Yes, I insist on using the absolute required minimum of Promise-style coding.
I don't know if we have any other example of a Promise-based API used - there might be an example somewhere on how to break away from it and go back to sane code.

I have to wait for the success or failure of the copy action, so I think this ui example makes no sense for my case. The notification should not be triggered by a button click.

Don't do that thing to the JS file. To add a notification, use the existing notification feature we already have - see https://we.phorge.it/uiexample/view/PhabricatorNotificationUIExample/ (https://we.phorge.it/source/phorge/browse/master/src/applications/uiexample/examples/PhabricatorNotificationUIExample.php).

I'm not against replacing the mechanism - It's just that the original task description doesn't specify why that would be good (i.e., what the benefit would be).
That's clear from the discussion now, but should generally be included in the ticket description - the "describe "what", not "how"" principle.

A big benefit of using client-side variables in CSS, is that one can use the browser's prefers-color-scheme media query to select light or dark mode based on the user's browser or system setting, automatically, and that the transition from light to dark mode, or to other accent/highlight color schemes, requires no reload.

Is it possible to check if anything is relying on this not working with negative numbers?

Well, I see a tremendous critism and scepticism concerning modern client side techniques here. The world of frontend tooling has changed tremendously in the last 5 years. And I understand concerns, but nobody has to write JavaScript for certain browsers anymore. Javelin for example tries to solve so many problems that Babel solves with one config entry. But that's okay and I like the architecture here and know we can't change everything at once.

Ok, I don't understand the specifics of css-vars, but if they're better, they're better.

The overwriting of variables with media queries could be configured inside the theming class. So possible breakpoints can be part of a theming API.

And my general approach is: when the client can do something (and that even better), why should the server do it?

@avivey CSS vars are a vital part of modern modularized theming. They are versatile, can be overwritten with media queries or per class/element basis. They are editable in browser dev tools. If you would ask me: this step is pretty important.

In T15736#15726, @avivey wrote:

to have a working dev server when developing js - at the moment I have to ./bin/celerity map after every change to js and css to see my changes...

Are you sure about that? I thought celerity map was only needed when adding a file (in dev mode).

also, adding this file in src/extensions/ will let you have css/js files from src/extensions/rsrc/ loaded automatically:

to have a working dev server when developing js - at the moment I have to ./bin/celerity map after every change to js and css to see my changes...

Looks to me that the existing system is better at providing Themes? It allows real code, inheritance, etc. to set variable values.
That is, I think Extension Themes are better off writing PHP Post-Processor rather then CSS vars.

In T15039#1358, @bekay wrote:

The clone repo modal could really use some copy buttons.

Preferably replacing the Read / Write Button.

I don't know if that is part of this task, but the global typeahead search should hide all disabled users, wikis and repos, shouldn't it? This is nothing you want as a fast suggestion...

I've better triaged my root problem.

So maybe this is just a matter of parsing that commit part "Depends-on:" since everything else seem already implemented to me. Partially related quiz in T15738

Just as a reminder for me: Using a css variable for the height of the tags would be best. I could be overwritten at any point.

Well, my idea would be:

• Updates map

• Fixes some js linting warnings

@aklapper Yeah, that would be great. Sorry, I don't have a working dev instance right now.

@deric: Hi, would you like to arc land your patch, or do you need any help? Thanks in advance!

For the records, adding public function getValueForQuery($value) is needed because otherwise Expected a numeric scalar or null for %d conversion. Query: project.projectDepth >= %d

Would anyone fancy reviewing this? :)

whoops

Make code more readable

In D25525#15322, @valerio.bozzolan wrote:

Maybe simplify if (empty($v)) { to just if ($v) { and swapping the case

In D25512#15325, @valerio.bozzolan wrote:

@sirocyl Does this fix your issue? if yes, I can help in landing

Heck yeah, changing phutil_nonempty_string() to phutil_nonempty_stringlike() there fixed the issue I had mentioned in a comment in T15737: Include information for installing required PHP version in Diviner docs.