Fix a typo

Done that. I left the technically-redundant methods where I have a lengthy comment explaining why I return en_US, since I think it's helpful to explain why that is.

Don't make unnecessary whitespace changes

Set fallback locale in base class

The current behavior if a fallback locale is not specified is to fallback to the proto-English string (English without applying pluralization). It probably does make sense, now that you bring it up, to fall back to en_US instead by default instead of doing that in every locale.

What's the current behavior for fallback from cs_CZ? Would it make sense to set the default to en_US in the base class?

{F2235244}

Ce a comment

This is not a full fix, still it is an improvement (and a base if anyone feels like fixing this perfectly).

Fix lint

revi confirmed this is called by getRepositorySlug() so abandoning this patch as hiding the underlying issue (no repository slug defined?) is not the right approach

In D25636#17849, @valerio.bozzolan wrote:

Nice catch. Basically the question is, why is this not working?

@revi: Could you please double-check and confirm that the line 144 in <phorge>/src/infrastructure/editor/PhabricatorEditorURIEngine.php is
'n' => $this->escapeToken($repository->getRepositorySlug()),
in your installation? Thanks in advance!

can it cause anything bad

What if $cdn has some evil characters? Do we have some escaping, or can it cause anything bad, or do we not care? Looks like we're including a bunch of other config as-is here, so maybe it's ok.

Looks good to me.

Good point, thanks! Updated.

Should the summary also include https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/rel/preconnect as a reference? https://developer.mozilla.org/en-US/docs/Web/Performance/dns-prefetch doesn't mention preconnect in the first few paragraphs, so it's somewhat confusing.

Also remove now unused private $objectPHID

Remove a needless ->setObjectPHID($object_phid) call I missed now that we pass $object

I still don't like the solution of using place-holder text for that

Thanks for the review (and for your patience!).

Updating D25693: Conduit API: allow phriction.edit to modify viewPolicy and editPolicy

I still don't like the solution of using place-holder text for that; But this implementation also adds another DB access which I think is redundant.

Instead of this diff, can you add a modern endpoint that uses the EditEngine/transactions? e.g. maniphest.edit, badge.edit, etc.
That will need a different name (phriction.document.edit?), but will allow full editing capability.

Thank you!

I have this look on Windows for

a {
  outline-style: auto;
}

Last Chrome	Last Firefox

This is the same as Q132: Why is there an `outline-style: none;` declaration for all links within Phorge/Phabricator?

Cleaner attempt in D25546 Diff 2113.

Allow to setEditEngine when constructing a PhabricatorApplicationTransactionCommentView

Thanks for the pointers, I really appreciate them.

Ah, thanks, makes sense now. I'd probably rephrase "but harms the authors constraint" a little bit to explicitly cover what you explained well in D25676#18549 - an author PHID in $this->responsibles might not always be a member of a project or owner of a package, so the query results were incomplete beforehand.

Ah I see below it assumes string. Thanks!

Is it possible tasks isn’t a string in this case, where non_empty_scalar might be more of a match?

seems good, ty!

Thanks. Moreover, looking at PhutilKeyValueCache it seems that setKey(null) has no sense. So thanks also for that extra care you already set.