Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TASK] Allow to load guides.xml when in composer mode #817

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

garvinhicking
Copy link
Contributor

When building a PHAR, or using this project as a composer dependency, the vendor/bin/guides binary could not be called due to a PHP error.

This is because the file guides.xml is referenced via vendor/../guides.xml. That works in the root of the main render-guides repository, but not in a composer environment.

There the correct location is vendor/t3docs/render-guides/guides.xml.

This PR adds a check to resolve that file in case the first try returns FALSE for the realpath() lookup.

Generally this lookup code is a bit spaghetti-ish and we should properly refactor this. For the time being, a hotfix like this would allow me to continue on using render-guides as a dependency in referring projects.

When building a PHAR, or using this project as a composer dependency,
the `vendor/bin/guides` binary could not be called due to a PHP error.

This is because the file `guides.xml` is referenced via
`vendor/../guides.xml`. That works in the root of the main
`render-guides` repository, but not in a composer environment.

There the correct location is `vendor/t3docs/render-guides/guides.xml`.

This PR adds a check to resolve that file in case the first try
returns FALSE for the realpath() lookup.

Generally this lookup code is a bit spaghetti-ish and we should
properly refactor this. For the time being, a hotfix like this
would allow me to continue on using `render-guides` as a dependency
in referring projects.
garvinhicking added a commit to garvinhicking/typo3-documentation-browsersync that referenced this pull request Jan 19, 2024
Some issues remain with render-guides, made a PR for that:

phpDocumentor/guides#817
@jaapio
Copy link
Member

jaapio commented Jan 21, 2024

@garvinhicking I just merged #824, does this solve your issue as well? the error might be gone, but I think you have a more specific use case for typo3, when the tool is installed via composer?

@fe-hicking
Copy link

No, doesnt fix mine because I need the fallback without ../ just like in this PR. Maybe you could look at it again, it doesn't do so much more than the one you merged apart from one more directory check...

@fe-hicking
Copy link

Ah, now I see the problem - I need to reference the guides.xml of a specific composer packe, but it's one that uses "guides" but it isn't guides itself.

So we'd probably need another configuration directive to specify the projectConfig file. Can I override that later on in the workflow, or do you have a suggestion how to specify the file without needing a patch so individual like mine?

@jaapio
Copy link
Member

jaapio commented Jan 22, 2024

Maybe we should solve this in some way in the typo3 project, let's see if that's possible someway.

@wouterj
Copy link
Contributor

wouterj commented Jan 22, 2024

The script tries to load the guides.xml file from 3 locations:

  • The directory containing vendor/ (i.e. the project root)
  • The working directory, either the current directory where the guides command is ran or the directory specified using the --working-dir option
  • From any other directory specified by --config

@garvinhicking
Copy link
Contributor Author

garvinhicking commented Jan 22, 2024

The script tries to load the guides.xml file from 3 locations:

  • The directory containing vendor/ (i.e. the project root)
  • The working directory, either the current directory where the guides command is ran or the directory specified using the --working-dir option
  • From any other directory specified by --config

Let me try to rephrase the problems I have.

The TYPO3 render-guides composer package (not really, because this is not offered as a composer package...) requires this phpdocumentor/guides. The guides.xml file of the render-guides repository is thus part of the "root" project.

When checking out the render-guides repository, the file will be loaded because it matches "vendor-dir/../guides.xml". All good so far. Additionally, another config file can additionally be loaded from a subdirectory (e.g. render-guides/Documentation/guides.xml). That sub-directory guides.xml only has documentation-specific configuration, while the main guides.xml has some core-level configuration.

core-level example:

<?xml version="1.0" encoding="UTF-8" ?>
<guides
    xmlns="https://www.phpdoc.org/guides"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="https://www.phpdoc.org/guides vendor/phpdocumentor/guides-cli/resources/schema/guides.xsd"
    links-are-relative="true"
    theme="typo3docs"
>
    <extension class="\phpDocumentor\Guides\RestructuredText\DependencyInjection\ReStructuredTextExtension"/>
    <extension class="\phpDocumentor\Guides\Bootstrap\DependencyInjection\BootstrapExtension"/>
    <extension class="\phpDocumentor\Guides\Graphs\DependencyInjection\GraphsExtension"/>
    <extension class="\phpDocumentor\Guides\Markdown\DependencyInjection\MarkdownExtension"/>
    <extension class="\T3Docs\Typo3DocsTheme\DependencyInjection\Typo3DocsThemeExtension"/>
    <extension class="\T3Docs\GuidesExtension\DependencyInjection\Typo3GuidesExtension"/>
    <extension class="\T3Docs\GuidesPhpDomain\DependencyInjection\GuidesPhpDomainExtension"/>
    <output-format>html</output-format>
    <output-format>singlepage</output-format>
    <output-format>interlink</output-format>
</guides>

project-level:

<?xml version="1.0" encoding="UTF-8" ?>
<guides
    xmlns="https://www.phpdoc.org/guides"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xsi:schemaLocation="https://www.phpdoc.org/guides vendor/phpdocumentor/guides-cli/resources/schema/guides.xsd"
>
    <project title="Render guides"/>
    <extension
        class="\T3Docs\Typo3DocsTheme\DependencyInjection\Typo3DocsThemeExtension"
        edit-on-github="TYPO3-Documentation/render-guides"
        edit-on-github-branch="main"
        interlink-shortcode="t3docsRenderGuides"
        typo3-core-preferred="stable"
    />
    <!-- explicitly link to stable versions of extensions -->
    <inventory id="georgringer/news/stable" url="https://docs.typo3.org/p/georgringer/news/11.3/en-us/"/>
    <inventory id="cobweb/external_import/stable" url="https://docs.typo3.org/p/cobweb/external_import/7.2/en-us/"/>
</guides>

Both files need to be loaded (not just one). If I read the code right, the first one is $projectConfig, and the second one is $localConfig in the packages/guides-cli/bin/guides script.

If I now create a package that uses the phar or composer package, when running it, the vendor/../guides.xml no longer resolves to the guides.xml that is specified in t3docs/render-guides. Because that is actually in vendor/t3docs/render-guides/guides.xml.

To get this all running, what we need is to be able to influence BOTh the $projectConfig AND $localConfig.

The current phpdocumentor/guides-cli implementation only makes $localConfig configurable, and NOT $projectConfig. When using the phar I could thus only either specify the guides.xml of my own project or the one of t3docs/render-guides -- but I would need both.

So in the end I would need the ability to specify:

  • vendorConfig = vendor/t3docs/render-guides/guides.xml
  • projectConfig = /pth/to/my/project/guides.xml
  • localConfig = /path/to/my/project/Documentation/guides.xml

Currently that first one is the one I need to "map" to t3docs/render-guides but it is not configurable. So on top of --working-dir and --config I would probably need --vendor-config?!

I hope this explanation is better understandable now.

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.

None yet

4 participants