Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.6k
[Console] Add markdown format to Table#59657
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation
carsonbot commentedJan 30, 2025
It looks like you unchecked the "Allow edits from maintainer" box. That is fine, but please note that if you have multiple commits, you'll need to squash your commits into one before this can be merged. Or, you can check the "Allow edits from maintainers" box and the maintainer can squash for you. Cheers! Carsonbot |
Are the failing tests related to my commit? |
| @@ -46,6 +46,8 @@ class TableStyle | |||
| private string $cellRowFormat = '%s'; | |||
| private string $cellRowContentFormat = ' %s '; | |||
| private string $borderFormat = '%s'; | |||
| private bool $intro = true; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't know what is a practice in Symfony to name boolean properties, but IMO it's more clear when they haveis* orhas* prefixes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
good point. Also I was thinking to only use one field "$hasFraming" and not allow setting intro and outro separately. Also I am not sure if "framing" is a good name.
Thoughts?
nicolas-grekas left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm failing to get what intro/outro concepts are for. Can you please update the description of your PR to show some examples? Maybe some test cases are missing also?
edit: please add a line in the changelog of the component also
Uh oh!
There was an error while loading.Please reload this page.
@nicolas-grekas naming is probably not so good, thanks for the suggestion. It's simply for omitting the first and the last line (------) of the table to be able to render proper markdown (see the test case) |
I changed the naming. Remark: fabbot.io is suggesting this patchhttps://fabbot.io/report/symfony/symfony/59657/20ecdd1a0473ceaa5fb792b900ab41877da35a53 but I did not change those lines (and changing them would break the PHPunit tests) |
Thank you@amenk. |
736ef64 intosymfony:7.3Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Aim: allow rendering markdown tables. Those don't have a Outside border frame line like --------, so we allow to remove them and define the format "markdown".
Our motivation is to pipe symfony console output in CI to GitHub/gitlab merge requests and format them properly.