Skip to content

Add choice element rendering in documentation#1613

Open
mretegan wants to merge 2 commits intonexusformat:mainfrom
XraySpectroscopy:docs-choice-support
Open

Add choice element rendering in documentation#1613
mretegan wants to merge 2 commits intonexusformat:mainfrom
XraySpectroscopy:docs-choice-support

Conversation

@mretegan
Copy link

@mretegan mretegan commented Feb 7, 2026

This PR adds support for rendering <choice> elements in the HTML documentation and ensures that all elements (fields, groups, choices, links) are rendered in their original XML document order. Closes #1439

@mretegan mretegan changed the title Add choice element rendering in documentationDocs choice support Add choice element rendering in documentation Feb 10, 2026
@woutdenolf
Copy link
Contributor

@PeterC-DLS Any idea why the CI does not get executed?

@PeterC-DLS
Copy link
Contributor

We have chosen to set the Approval rule for running fork PR workflows to "Only users who have never had a commit or pull request merged into this repository will require approval to run workflows" - see (https://docs.github.com/en/actions/how-tos/manage-workflow-runs/approve-runs-from-forks)

@woutdenolf
Copy link
Contributor

I don't seem to have permissions to give approval unless I'm missing something.

@PeterC-DLS
Copy link
Contributor

Perhaps they can try rebasing it

@woutdenolf
Copy link
Contributor

The "choice" docs are now showing up like this:

image

@woutdenolf woutdenolf self-requested a review February 11, 2026 14:00
@woutdenolf woutdenolf self-requested a review February 11, 2026 14:10
"\n"
)
# Process children in document order to preserve XML ordering.
for node in parent.xpath("nx:field|nx:group|nx:choice|nx:link", namespaces=ns):
Copy link
Contributor

@woutdenolf woutdenolf Feb 11, 2026

Choose a reason for hiding this comment

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

The diff is very hard to read but as I understand instead of doing a for-loop for each tag, we do one loop for all tags and use an if-statement to distinguish between tags. And we do this to keep the order as defined in the XML file.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mretegan Why do we need to preserve the order here?

Copy link
Author

@mretegan mretegan Feb 12, 2026

Choose a reason for hiding this comment

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

It felt like the normal thing to do. There is always some logic behind the order in which people list things in the XML, so I think it should be kept in the HTML rendering. When I first added the <choice> rendering, I was surprised to see it at the end.

Copy link
Contributor

@woutdenolf woutdenolf Feb 12, 2026

Choose a reason for hiding this comment

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

There is always some logic behind the order in which people list things, so I think it should be kept in the HTML rendering.

But it could have been an explicit choice to systematically group fields, groups, links etc. in the HTML docs. The order within each group is still kept. Do you know @PeterC-DLS @prjemian ?

Copy link
Contributor

@woutdenolf woutdenolf left a comment

Choose a reason for hiding this comment

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

Apart from refactoring the way we extract tag names (group, field, attribute, link, choice) from XML nodes it looks good.

Edit: I'll refactor the tag stuff in another PR.

@woutdenolf
Copy link
Contributor

woutdenolf commented Feb 12, 2026

@PeterC-DLS This is a fix for showing choice tags in the HTML docs. Do you mind we merge this before the patch release or wait till after (better safe than sorry)?

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.

choice fields are not rendered in HTML

3 participants