Add a new parameter max_width to MyPrettyTable
#2426
Conversation
…ay for CLI usage This PR adds a `max_width` parameter to MyPrettyTable to restrict the maximum width of its underlying table. The value can be an integer, "max" (detect automatically the correct width) or None (no width limit)
|
Warning Rate limit exceeded@0xalpharush has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 47 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe modifications primarily focus on enhancing the Changes
Sequence DiagramssequenceDiagram
participant User
participant CommandLine
participant MyPrettyTable
User->>CommandLine: Executes command
CommandLine->>MyPrettyTable: Initializes table with max_width="max"
MyPrettyTable->>System: Checks terminal size
System-->>MyPrettyTable: Returns terminal size
MyPrettyTable->>CommandLine: Prepares table with adjusted width
CommandLine->>User: Displays formatted table output
sequenceDiagram
participant User
participant CommandLine
participant LocPrinter
User->>CommandLine: Requests help information
CommandLine->>LocPrinter: Fetches HELP attribute
LocPrinter->>CommandLine: Returns cleaned HELP string
CommandLine->>User: Displays formatted help information
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
|
I would keep this onhold until prettytable is upgraded upstream. |
0xalpharush
requested review from
montyly,
0xalpharush and
smonicas
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
setup.py (1)
Line range hint
3-3: Remove unnecessary open mode parameters.- with open("README.md", "r", encoding="utf-8") as f: + with open("README.md", encoding="utf-8") as f:slither/utils/command_line.py (2)
Line range hint
126-126: Convert tonot infor membership tests to improve readability.- if not filter_wiki in detector.WIKI: + if filter_wiki not in detector.WIKI:Also applies to: 240-240
Line range hint
157-157: Consider renaming the variablelto something more descriptive to avoid ambiguity.- for l in lines: + for line in lines:Also applies to: 171-171
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- setup.py (1 hunks)
- slither/printers/summary/loc.py (1 hunks)
- slither/utils/command_line.py (2 hunks)
- slither/utils/myprettytable.py (5 hunks)
Files skipped from review due to trivial changes (1)
- slither/printers/summary/loc.py
Additional context used
Ruff
setup.py
3-3: Unnecessary open mode parameters (UP015)
Remove open mode parameters
slither/utils/command_line.py
126-126: Test for membership should be
not in(E713)Convert to
not in
157-157: Ambiguous variable name:
l(E741)
171-171: Ambiguous variable name:
l(E741)
240-240: Test for membership should be
not in(E713)Convert to
not in
Additional comments not posted (3)
setup.py (1)
16-16: Update toprettytableversion requirement is aligned with the PR's objectives.slither/utils/myprettytable.py (1)
Line range hint
11-37: The implementation of themax_widthparameter inMyPrettyTableis well-handled, including terminal size detection.slither/utils/command_line.py (1)
275-277: The use of themax_width="max"parameter in output functions aligns with the PR's objectives to improve table formatting.Also applies to: 359-359
Co-authored-by: dm <alexis.challande@trailofbits.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (2)
- slither/utils/command_line.py (1 hunks)
- slither/utils/myprettytable.py (5 hunks)
Additional context used
Ruff
slither/utils/myprettytable.py
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
slither/utils/myprettytable.py
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
Additional comments not posted (5)
slither/utils/myprettytable.py (4)
11-16: LGTM! Ensure clarity ofmax_widthlogic.The addition of the
max_widthparameter and the logic for setting it based on terminal size is correctly implemented. The comment about reducing terminal columns by 3 is helpful for understanding the logic.
28-36: LGTM! Logic for settingmax_widthis sound.The logic for setting
max_widthbased on terminal size is well-implemented and accounts for potential inconsistencies in terminal sizes.
47-48: LGTM! Dynamic table width adjustment.The changes to the
to_pretty_tablemethod ensure that the table width is dynamically adjusted based on themax_widthparameter.
85-85: LGTM! Addition of totals row.The change to add a totals row if the
totalsparameter is set toTrueis correctly implemented.slither/utils/command_line.py (1)
363-364: LGTM! Proper formatting of multi-line help information.The change to clean up multi-line help information before adding it to the table ensures that the help information is properly formatted when displayed.
slither/utils/myprettytable.py
Outdated
| @@ -1,4 +1,5 @@ | |||
| from typing import List, Dict, Union | |||
| from shutil import get_terminal_size | |||
| from typing import List, Dict, Union, Literal | |||
There was a problem hiding this comment.
Remove unused import.
The Literal import from typing is unused and should be removed to clean up the code.
- from typing import List, Dict, Union, Literal
+ from typing import List, Dict, UnionCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from typing import List, Dict, Union, Literal | |
| from typing import List, Dict, Union |
Tools
Ruff
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
slither/utils/myprettytable.py (2)
28-35: Clarify the reduction of terminal columns by 3.Add a comment to explain why the terminal columns are reduced by 3.
+ # Reduce terminal_column by 3 to account for potential padding or margin inconsistencies
48-50: Issue Found:max_table_widthAttribute Not Supported byprettytableLibraryThe
max_table_widthattribute set inslither/utils/myprettytable.pyis not supported by theprettytablelibrary, as there is no evidence of its existence in the library's source code. This could lead to runtime errors.
slither/utils/myprettytable.py, line 48-50Please verify the correct attribute or remove this line if unnecessary.
Analysis chain
LGTM! But verify the usage of
max_table_width.The code changes are approved.
However, ensure that the
max_table_widthsetting is correctly used and supported by theprettytablelibrary.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `max_table_width` in the `prettytable` library. # Test: Search for the `max_table_width` attribute in the `prettytable` library. Expect: Correct usage and support. rg --type python -A 5 $'max_table_width'Length of output: 70
Script:
#!/bin/bash # Description: Verify the usage of `max_table_width` in the `prettytable` library. # Test: Search for the `max_table_width` attribute in the `prettytable` library. Expect: Correct usage and support. rg --type py -A 5 $'max_table_width'Length of output: 455
Script:
#!/bin/bash # Description: Verify the usage of `max_table_width` in the `prettytable` library. # Test: Search for the `max_table_width` attribute in the `prettytable` library source code. Expect: Correct usage and support. rg --type py -A 5 'max_table_width' $(python3 -c "import prettytable; print(prettytable.__path__[0])")Length of output: 597
Script:
#!/bin/bash # Description: Verify the usage of `max_table_width` in the repository. # Test: Search for the `max_table_width` attribute in the entire repository. Expect: Correct usage and support. rg --type py -A 5 'max_table_width'Length of output: 455
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- slither/utils/myprettytable.py (5 hunks)
Additional context used
Ruff
slither/utils/myprettytable.py
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
slither/utils/myprettytable.py
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
Additional comments not posted (5)
slither/utils/myprettytable.py (5)
2-2: Remove unused import.The
Literalimport fromtypingis unused and should be removed to clean up the code.Tools
Ruff
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
38-38: LGTM!The
add_rowmethod is simple and correct.
Line range hint
58-60: LGTM!The
to_jsonmethod is simple and correct.Tools
Ruff
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
Line range hint
62-64: LGTM!The
__str__method is simple and correct.Tools
Ruff
2-2:
typing.Literalimported but unusedRemove unused import:
typing.Literal(F401)
GitHub Check: Lint Code Base
[warning] 2-2:
W0611: Unused Literal imported from typing (unused-import)
86-86: LGTM! But verify the initialization ofMyPrettyTable.The code changes are approved.
However, ensure that the
MyPrettyTableinitialization is correct and compatible with the newmax_widthparameter.
Add a new parameter
max_widthto MyPrettyTable to enhance its display for CLI usageThis PR adds a
max_widthparameter to MyPrettyTable to restrict the maximum width of its underlying table.The value can be an integer, "max" (detect automatically the correct width) or None (no width limit)
The output is better, and will be even better once
prettytablewill be upgraded to 3.10.1 on PyPihttps://github.com/jazzband/prettytable/releases/tag/3.10.1
Summary by CodeRabbit
New Features
MyPrettyTablenow dynamically adjusts based on terminal size with the newmax_widthparameter.Bug Fixes
Chores
prettytablepackage requirement to version3.10.2for better compatibility and features.