Skip to content

Add support for extension configuration #482

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

Merged
merged 2 commits into from
Sep 5, 2023
Merged

Conversation

wouterj
Copy link
Contributor

@wouterj wouterj commented Jul 7, 2023

Allows per-extension configuration:

<?xml version="1.0" encoding="UTF-8" ?>
<guides>
    <extension class="phpDocumentor\Guides\Code">
        <highlighter-language label="php+attribute">php</highlighter-language>
    </extension>
</guides>

And global configuration (which underneeds gets automatically applied to the GuidesExtension):

<?xml version="1.0" encoding="UTF-8" ?>
<guides>
    <project title="Project Title" version="6.4"/>
</guides>

@greg0ire
Copy link
Contributor

greg0ire commented Jul 7, 2023

I think the XML snippet in the PR description could be used in https://github.com/phpDocumentor/guides/blob/main/docs/configuration.rst

@linawolf
Copy link
Contributor

@wouterj could we integrate intersphinx configuration (#494) into this somehow?

@linawolf
Copy link
Contributor

@wouterj @greg0ire what is needed to make this work?

@greg0ire
Copy link
Contributor

@linawolf I don't understand the question. Are you saying it does not work?

@linawolf
Copy link
Contributor

linawolf commented Aug 22, 2023

@greg0ire Some tests are still red and it needs to be rebased so we can merge it

@greg0ire
Copy link
Contributor

Ok: what needs IMO to be done for this to be mergeable:

  • fix the cs issue
  • either address or ignore the static analysis issue
  • add documentation
  • add tests if possible

@linawolf
Copy link
Contributor

@wouterj @greg0ire I was trying to add a functional test, however it seems like it has not effect. I don't really understand how this is supposed to work so could someone ™️ support me here?

@linawolf linawolf force-pushed the config branch 2 times, most recently from 114e81a to 57b901e Compare August 25, 2023 07:53
@greg0ire
Copy link
Contributor

This PR allows to build a configurable highlighter, but does not build it itself, that's part of what #475 is for.

@linawolf
Copy link
Contributor

@greg0ire is there anything that we can already test with an integration test? I still don't understand how this works or how to use the configuration. Not even if my example file is named correctly and in the correct place

@wouterj
Copy link
Contributor Author

wouterj commented Aug 25, 2023

The second snippet with <project title="Project Title" version="6.4"/> is already implemented.

Each extension defines its configuration in the getConfigTreeBuilder() method: https://github.com/phpDocumentor/guides/pull/482/files#diff-a98cf5815ab9d23df43257abd002a77c5792f02cff4ef0ee5add504533a9d8cbR36

The Config component parses the XML and finds the nested nodes from the config tree based on this. So for a <project> XML element, it'll find the arrayNode('project') and the title="..." attribute is mapped to the scalarNode('title') child.

This is then processed by the Config component (https://github.com/phpDocumentor/guides/pull/482/files#diff-a98cf5815ab9d23df43257abd002a77c5792f02cff4ef0ee5add504533a9d8cbR71-R72) and transformed into one merged associative array like so:

[
  'project' => [
    'title' => '...',
  ]
]

This doc is useful if you want to learn more about this processing: https://symfony.com/doc/current/components/config/definition.html

Finally, you can use <extension class="phpDocumentor\Guides\Code"> to determine which extension's "config tree" will be searched for. E.g. in this example, the child elements will be parsed using the CodeExtension (which does not exist yet). All elements that are in the root of the document and are not <extension> are parsed using the GuidesExtension (to make it easier to configure the built-in config).

@linawolf
Copy link
Contributor

@wouterj, I updated the integration test accordingly, unfortunate it does not seem to work

@greg0ire
Copy link
Contributor

greg0ire commented Sep 2, 2023

@linawolf it looks like this is the first time a guides.xml file is created in a test. The only place where guides.xml is referenced is here:

if (is_file(getcwd().'/guides.xml')) {
$containerFactory->addConfigFile('guides.xml');
}

but the CLI does not get run when in tests, right? So I don't think the guides.xml in this PR will be taken into account at all.

@linawolf
Copy link
Contributor

linawolf commented Sep 3, 2023

@greg0ire so I am loading the guides.xml in a test now if it is present, however it still does not seem to do anything...

@wouterj
Copy link
Contributor Author

wouterj commented Sep 3, 2023

Fyi: I'm taking a look now

@wouterj wouterj force-pushed the config branch 2 times, most recently from 905ca5a to ad1d217 Compare September 3, 2023 21:45
@wouterj
Copy link
Contributor Author

wouterj commented Sep 3, 2023

Fixed the tests, added some docs, added the XML schema and did some other minor improvements to the project settings flow.

I think this is ready now.

@linawolf
Copy link
Contributor

linawolf commented Sep 4, 2023

Looks good to me, thanks a lot for these changes

wouterj and others added 2 commits September 5, 2023 08:35
I did not manage to fix the one error so I put it in the baseline for now
@jaapio jaapio merged commit 1663716 into phpDocumentor:main Sep 5, 2023
@wouterj wouterj deleted the config branch September 5, 2023 08:16
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.

4 participants