Skip to content

feat: refactor env command to use ui.display_table for consistent table display#286

Merged
bigcat88 merged 3 commits into
Comfy-Org:mainfrom
neverbiasu:env
Aug 2, 2025
Merged

feat: refactor env command to use ui.display_table for consistent table display#286
bigcat88 merged 3 commits into
Comfy-Org:mainfrom
neverbiasu:env

Conversation

@neverbiasu

Copy link
Copy Markdown
Contributor

What changed

Updated the comfy env command to use our existing ui.display_table() function instead of manually creating Rich Table objects.

Why

  • Keeps table rendering consistent across the codebase
  • Follows the pattern we already use elsewhere
  • Makes the code cleaner by separating data from presentation

Changes made

  • Modified EnvChecker.fill_print_table() to return data tuples instead of creating a table
  • Updated WorkspaceManager.fill_print_table() to return data tuples too
  • Changed the env command to collect all data first, then render it with ui.display_table()

The output looks exactly the same to users.

after before
after before

@dosubot dosubot Bot added size:M This PR changes 30-99 lines, ignoring generated files. enhancement New feature or request labels Jun 18, 2025
@robinjhuang robinjhuang requested a review from bigcat88 July 26, 2025 22:44
@robinjhuang

Copy link
Copy Markdown
Member

Seems like a reasonable change. @bigcat88 Can you take a look?

@bigcat88 bigcat88 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Overall the PR is good (tested locally), but some small changes need to be made.

Comment thread comfy_cli/workspace_manager.py Outdated
Comment thread comfy_cli/env_checker.py Outdated
Comment thread comfy_cli/cmdline.py Outdated
Comment thread comfy_cli/cmdline.py
Comment thread comfy_cli/config_manager.py Outdated
@codecov

codecov Bot commented Jul 29, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 26.47059% with 25 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
comfy_cli/config_manager.py 33.33% 10 Missing ⚠️
comfy_cli/env_checker.py 0.00% 10 Missing ⚠️
comfy_cli/cmdline.py 42.85% 4 Missing ⚠️
comfy_cli/workspace_manager.py 50.00% 1 Missing ⚠️
Flag Coverage Δ
unittests 47.96% <26.47%> (+2.39%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
comfy_cli/workspace_manager.py 63.41% <50.00%> (+7.31%) ⬆️
comfy_cli/cmdline.py 47.67% <42.85%> (-0.17%) ⬇️
comfy_cli/config_manager.py 50.00% <33.33%> (-4.80%) ⬇️
comfy_cli/env_checker.py 46.80% <0.00%> (-2.13%) ⬇️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bigcat88

Copy link
Copy Markdown
Contributor

thanks for the changes, can you remove the last unused import that ruff complains about so we can merge this?

@neverbiasu

Copy link
Copy Markdown
Contributor Author

sure, just removed it

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Aug 1, 2025
@bigcat88 bigcat88 merged commit 2575000 into Comfy-Org:main Aug 2, 2025
11 of 14 checks passed
@bigcat88

bigcat88 commented Aug 2, 2025

Copy link
Copy Markdown
Contributor

thanks for this contribution

@Arcitec

Arcitec commented Aug 2, 2025

Copy link
Copy Markdown
Contributor

Nice cleanup and unification. Good job! 👍

@neverbiasu

Copy link
Copy Markdown
Contributor Author

thx guys🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants