Skip to content

[skip-revcheck] Remove empty xi:fallback elements#5699

Open
lacatoire wants to merge 2 commits into
php:masterfrom
lacatoire:chore/remove-xi-fallback
Open

[skip-revcheck] Remove empty xi:fallback elements#5699
lacatoire wants to merge 2 commits into
php:masterfrom
lacatoire:chore/remove-xi-fallback

Conversation

@lacatoire

@lacatoire lacatoire commented Jul 23, 2026

Copy link
Copy Markdown
Member

Removes empty <xi:fallback/> across doc-en, turning
<xi:include ...><xi:fallback/></xi:include> into <xi:include .../>.

An empty fallback replaces a failed XInclude with nothing, hiding the failure
from the reporting configure.php already gives.

The two <xi:fallback> inside example CDATA (dom xinclude /
getElementsByTagNameNS) are kept, they document the feature itself.

Related:

[skip-revcheck]: no translated content affected.

An empty <xi:fallback/> replaces a failed XInclude with nothing, hiding the
failure from the reporting configure.php already provides. Continues #274 and
the doc-base roadmap (#199). Imagick exception classes are handled separately,
they need a content fix to their xpointers.

An empty node set is swallowed by libxml 2.15 but surfaces as a failed
XInclude on libxml 2.9 (CI), so only CI validates this.
These 11 fallbacks injected an empty <varlistentry> when their include failed,
rendering a garbage parameter row instead of surfacing the failure.
xinclude_residual_fixup already covers the variablelist parent, so the
hand-written net is redundant. The only fallbacks kept are the two inside
example CDATA (dom xinclude/getElementsByTagNameNS), which document XInclude.
@alfsb

alfsb commented Jul 23, 2026

Copy link
Copy Markdown
Member

This is very impressive. And also very tasteful (the /> thing).

This was generated by script? If so, then can  you publish it somewhere, or can provide a patch file for translations, after this is merged? doc-base/languages.php is an easy way to generate a fresh clone of all languages in a new dir, for experiments.

As <xi:fallback> now causes more problems that it solves, I plan to merge this somewhat fest, by next monday at most, to avoid conflict to creep in. I will do a heads up on Discord, and also reserve some time to do a full testing for XML or revcheck differences. This will be a good testing bed for consecutive skip-revcheck code.

@alfsb

alfsb commented Jul 23, 2026

Copy link
Copy Markdown
Member

Wow. You even found the xi:fallbacks with inline bodies. Nice!

@jordikroon
jordikroon requested review from Girgias, alfsb and jordikroon and removed request for kamil-tekiela and ndossche July 23, 2026 16:43

@jordikroon jordikroon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I scanned through randomly and don't see anything weird. Also assuming the build will fail if something is wrong so I trust this is alright.

@alfsb

alfsb commented Jul 23, 2026

Copy link
Copy Markdown
Member

I scanned through randomly and don't see anything weird. Also assuming the build will fail if something is wrong so I trust this is alright.

GH CI cannot be really trusted for this, as it does not consider posterior changes on main/master. So an ok result on CI can (and frequently is) completely broken after merge.

@alfsb

alfsb commented Jul 23, 2026

Copy link
Copy Markdown
Member

I scanned through randomly and don't see anything weird. Also assuming the build will fail if something is wrong so I trust this is alright.

GH CI cannot be really trusted for this, as it does not consider posterior changes on main/master. So an ok result on CI can (and frequently is) completely broken after merge.

See, for example, on tests of this PR: https://github.com/php/doc-en/actions/runs/29993851297/job/89162721067?pr=5699

In "Checkout php/doc-en" step, we can see

HEAD is now at 4ff27bc Merge 6f4e802c5fd1e8c698e7836da8b88882947b00bf into e1b9f588815a51682acf68f9b8929f49ad612488

Note that e1b9 is NOT the head of doc-en, which now sits at 07b2, after e1b9.

In "Build documentation for en" we can see that doc-en is in 4ff2, a merge hash yes, but a merge hash into the past of doc-en.

doc-base: 9391318e46e6fdd09721a9f5e3553089e4382009
en:       4ff27bc6fb5413930ea05d1348f6de1067c808a7

I suspect the null in pull_request: null may have something to it, as it does not show in any GitHub examples.

I did not re-run the tests, to see if it changes anything. If anyone re-runs, and the hashes change, please comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants