Skip to content

entityresolver: resolve Windows absolute paths - #58

Merged
jordikroon merged 1 commit into
php:mainfrom
vrana:windows-absolute-paths
Oct 23, 2026
Merged

jordikroon merged 1 commit into
php:mainfrom
vrana:windows-absolute-paths

Conversation

@vrana

@vrana vrana commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

SYSTEM references in doc-base's doctype.dtd are absolute, which on Windows means C:/... . Only paths starting with / were treated as absolute, so these were appended to the including file's directory and entities like &reference.pdo.entities.pdostatement; were reported as not defined, hiding all other violations in the file.

Treat drive-letter and UNC paths as absolute too.

SYSTEM references in doc-base's doctype.dtd are absolute, which on
Windows means C:/... . Only paths starting with / were treated as
absolute, so these were appended to the including file's directory and
entities like &reference.pdo.entities.pdostatement; were reported as
not defined, hiding all other violations in the file.

Treat drive-letter and UNC paths as absolute too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jordikroon

jordikroon commented Oct 23, 2026 •

Copy link
Copy Markdown
Member

Thank you for this. This tool hasn't been tested on Windows yet, but I am happy to support it.
The change looks good to me. Could you fix the remaining PHP Stan issues?

Main now also has windows included in the CI pipeline.

@vrana
vrana force-pushed the windows-absolute-paths branch from b4c5c89 to 21e004b Compare October 23, 2026 08:44
@vrana

vrana commented Oct 23, 2026

Copy link
Copy Markdown
Contributor Author

It fails on main: #59

@jordikroon
jordikroon merged commit 3eaa743 into php:main Oct 23, 2026
3 of 6 checks passed
@jordikroon

Copy link
Copy Markdown
Member

The tests will still fail after this change. If you wish to do a follow-up PR you are very welcome to do. If not I will get to it in the upcoming days when I get my hands on Windows.

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.

2 participants