💄 Unify the width of the Rich console for help and errors#788
💄 Unify the width of the Rich console for help and errors#788tiangolo merged 3 commits intofastapi:masterfrom
Conversation
|
@svlandeg hi, do you have some ETA to get this merged? Or is there something else I should/could do to get this merged? |
|
@svlandeg, what is blocking this PR ? We would like to finish the circle on this fix so our users have a better experience. Thank you |
This comment was marked as spam.
This comment was marked as spam.
svlandeg
changed the title
svlandeg
left a comment
svlandeg
left a comment
There was a problem hiding this comment.
Hi @racinmat, thanks for this PR!
It makes a lot of sense to unify the Rich console appearence, and call the rich_utils._get_rich_console() function to define the console_stderr. This seems to be the original intention as well, as this function takes an explicit stderr boolean argument.
Setting the width to MAX_WIDTH instead of taking the default of 100 also makes more sense to me, and it indeed looks nicer on the console.
I can confirm on my system that the width of the help info box and the width of the error box differ on master but are the same with this PR. It definitely comes across as more unified and clean this way.
Finally - on the point of whether or not this is breaking change and would require a feature switch: while the default behaviour does change, it's not really a breaking API change IMO, so I would probably merge this as-is. I'll leave the final decision on this with Tiangolo though.
tiangolo
changed the title
4 participants
Fixes #787 by unifying the behavior of consoles and passing the
widthparameter consistently.As this will change the default behavior, should I make some feature flag so the new behavior is opt-in, or is it ok like this?