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

Help panel sorted by name, allow to not expand help command tables #489

Closed
wants to merge 4 commits into from

Conversation

asaintsever
Copy link

@asaintsever asaintsever commented Nov 5, 2022

This PR brings following changes and features:

  • Rich Help Panels for commands are now sorted by panel name (except for 'Commands' panel always printed first). Currently, command help panels are sorted by command name (ordering being done across all panels) which may results in weird/unexpected display.
  • New STYLE_COMMANDS_TABLE_EXPAND style to allow to not expand tables of command help panels. The currently forced table expand may not be desirable when you have several command panels (help text may end up far on the right side and alignment with other panel text too inconsistent). Disabling tables' expand option helps improving the result (in combination with STYLE_COMMANDS_TABLE_PADDING style).

@github-actions
Copy link

github-actions bot commented Nov 6, 2022

📝 Docs preview for commit 98a0f3f at: https://6367f2806084a07b322a00ed--typertiangolo.netlify.app

@github-actions
Copy link

📝 Docs preview for commit 46a2868 at: https://639cea0318290c0c34ff487a--typertiangolo.netlify.app

@ssbarnea
Copy link
Contributor

ssbarnea commented Mar 13, 2023

Is this related to Textualize/rich#2878 and #567 ?

@asaintsever
Copy link
Author

@ssbarnea yep after looking at your discussion and PR, clearly related. Seems you also stumbled upon same issue with labels ending up not aligned across panels. Your proposal is different than mine though since I only make visible the expand property to fix it.

@ssbarnea
Copy link
Contributor

Now we need to find a way to persuade @tiangolo to pick one of the fixes, so we can make use of it.

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I had a look at this PR and compared it with master and #567. (don't mind the misaligned right-hand side, it's a c/p issue, it's actually nicely aligned on my console)

On master, I'd get:

┌─ Commands ──────────────────────────────────────────────────────────────────┐
│ create-a-wonderful-new-user                Create a new user. ✨            │
│ delete                                     Delete a user. 🔥                │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Utils and Configs ─────────────────────────────────────────────────────────┐
│ config    Configure the system. 🔧                                          │
│ sync      Synchronize the system or something fancy like that. ♻            │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Help and Others ───────────────────────────────────────────────────────────┐
│ help            Get help with the system. ❓                                │
│ report          Report an issue. 🐛                                         │
└─────────────────────────────────────────────────────────────────────────────┘

On this PR, with STYLE_COMMANDS_TABLE_EXPAND = True I'd get:

┌─ Commands ──────────────────────────────────────────────────────────────────┐
│ create-a-wonderful-new-user  Create a new user. ✨                          │
│ delete                       Delete a user. 🔥                              │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Help and Others ───────────────────────────────────────────────────────────┐
│ help    Get help with the system. ❓                                        │
│ report  Report an issue. 🐛                                                 │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Utils and Configs ─────────────────────────────────────────────────────────┐
│ config  Configure the system. 🔧                                            │
│ sync    Synchronize the system or something fancy like that. ♻              │
└─────────────────────────────────────────────────────────────────────────────┘

This is less white space pushing the description to the right, but still the commands aren't outlined across panels.

With PR #567, I'd get:

┌─ Commands ──────────────────────────────────────────────────────────────────┐
│ create-a-wonderful-new-user   Create a new user. ✨                         │
│ delete                        Delete a user. 🔥                             │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Utils and Configs ─────────────────────────────────────────────────────────┐
│ config                        Configure the system. 🔧                      │
│ sync                          Synchronize the system or something fancy     │
│                               like that. ♻                                  │
└─────────────────────────────────────────────────────────────────────────────┘
┌─ Help and Others ───────────────────────────────────────────────────────────┐
│ help                          Get help with the system. ❓                  │
│ report                        Report an issue. 🐛                           │
└─────────────────────────────────────────────────────────────────────────────┘

Which is nicely outlined across panels but does include a bit more whitespace, pushing e.g. the sync description to 2 lines instead of 1.

All in all, I think the output of this PR is better than what we'd have on master, but the result of PR #567 seems even more preferable. This is mostly a personal preference though.

In terms of functionality, this PR has two additional features to be considered:

I'll leave the final decision on this with Tiangolo!

@@ -59,6 +59,7 @@
STYLE_COMMANDS_TABLE_LEADING = 0
STYLE_COMMANDS_TABLE_PAD_EDGE = False
STYLE_COMMANDS_TABLE_PADDING = (0, 1)
STYLE_COMMANDS_TABLE_EXPAND = True
Copy link
Member

Choose a reason for hiding this comment

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

Having this as a separate option that by default behaves as before, is in itself a clean solution!

@tiangolo
Copy link
Member

tiangolo commented Apr 8, 2024

Thanks @asaintsever! 🚀

I went for #567 to handle the length of the commands.

I want to allow making the order of panels and commands explicit, but not sorted by order. I also want to change that for commands, so that developers can decide which commands are shown first in the help menus. I think commands and their help are in many cases related, or some should be used first, before others, etc. And that doesn't depend on the name but on the intention of the developer, so, I want to order commands in the same order they were added to the app instead of the alphabetically. 🤓

Given that, I'll pass on this one, but thanks for the effort! 🍰

@tiangolo tiangolo closed this Apr 8, 2024
@johnthagen
Copy link

@tiangolo Just for clarification:

I want to allow making the order of panels and commands explicit, but not sorted by order

This isn't currently possible, correct? Just wanting to make sure I hadn't missed this being implemented

Thanks ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature, enhancement or request p3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants