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

fix expression language precedence cell grouping #19894

Merged
merged 1 commit into from
May 21, 2024

Conversation

homersimpsons
Copy link
Contributor

Fix for #19860 (comment)

I do not know how to test the changes as those were working with the doc builder previously.

@OskarStark
Copy link
Contributor

Thank you @homersimpsons.

@OskarStark OskarStark merged commit f3356f4 into symfony:6.4 May 21, 2024
3 checks passed
@homersimpsons homersimpsons deleted the patch-2 branch May 21, 2024 18:33
@homersimpsons
Copy link
Contributor Author

@javiereguiluz Looks like this change did not work. I'm wondering how can I test the change before it goes live? What is the official site using? Why the rendering is that much different?

@javiereguiluz
Copy link
Member

@homersimpsons that's odd because you applied the same trick as I did in my PR. The table I changed is being rendered as expected. See https://symfony.com/doc/current/notifier.html#sms-channel

I can't see any difference in your RST table ... but when rendered it doesn't work as expected: https://symfony.com/doc/current/reference/formats/expression_language.html#operators-precedence

The best way to render the docs locally is to:

  • Clone this repo
  • cd _build/
  • composer install
  • php build.php

The generated docs use a very different design ... but just browse the page reference/formats/expression_language.html and check if the HTML structure of the generated table is correct.

@homersimpsons
Copy link
Contributor Author

I can't see any difference in your RST table

Technically speaking the main difference is that in your changes this was the last column while in mine it is the first column

The best way to render the docs locally is to

This is what I did for my first pull request. And it looked just fine (there were no additional rows)

check if the HTML structure of the generated table is correct

I did not check the HTML structure, I will check this.

xabbuh added a commit that referenced this pull request Jun 8, 2024
This PR was submitted for the 7.1 branch but it was merged into the 6.4 branch instead.

Discussion
----------

:art: Fix precedence table rendering

Use rst grid to get the correct layout. `list-table` directive is not supported.

This is a follow-up of #19894 which itself was a follow-up of !19860.

The rendered HTML is now correct

/cc `@OskarStark` `@javiereguiluz`

Commits
-------

f6b21f4 🎨 Fix precedence table rendering
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants