Skip to content

Use "package:foo" instead of foldernames in command execution #5801

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.

Sign up for GitHub

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

Jump to bottom
Merged
DanTup merged 1 commit into from
Nov 17, 2025
Merged

Use "package:foo" instead of foldernames in command execution #5801

DanTup merged 1 commit into from
Nov 17, 2025
+43 −31

Conversation

@DanTup
Copy link
Member

@DanTup DanTup commented Nov 17, 2025

Fixes #5789

@DanTup DanTup added this to the v3.124.0 milestone Nov 17, 2025
@DanTup DanTup added is enhancement in commands Relates to commands (usually invoked from the command Palette) labels Nov 17, 2025
@DanTup DanTup requested a review from Copilot November 17, 2025 16:56
@DanTup
Copy link
Member Author

DanTup commented Nov 17, 2025

@codex review
/gemini review

@gemini-code-assist
Copy link

gemini-code-assist bot commented Nov 17, 2025

Hello DanTup,

Thank you for requesting a review. I've thoroughly examined the changes in this pull request.

Overall Assessment:

This pull request introduces a valuable improvement by prioritizing the use of package:foo style names over raw folder names in command execution output. This change significantly enhances clarity and user experience, especially in multi-package repositories, by providing more contextually relevant identifiers for operations. The fallback mechanism to folder names ensures robustness when a pubspec.yaml is not found or cannot be parsed.

Review Criteria Adherence:

  • Correctness: The logic for determining packageOrFolderDisplayName is sound, correctly attempting to extract the package name and falling back to folder-based naming. The tryGetPackageName function includes appropriate error handling, returning undefined gracefully. The updated tests confirm the expected output changes.
  • Efficiency: The introduction of reading pubspec.yaml via tryGetPackageName is a minor overhead that is acceptable given the user-initiated nature of these commands and the significant improvement in output clarity.
  • Maintainability: The code is well-structured. The tryGetPackageName function is appropriately placed and exported from fs.ts. The variable packageOrFolderDisplayName clearly communicates its purpose. The conditional logic for channelName in runCommandInFolder is a thoughtful detail that prevents redundant output.

Conclusion:

This is a well-implemented and beneficial change. The code is clean, correct, and improves the overall user experience. I approve of these changes.

Great work!

Copilot AI reviewed Nov 17, 2025
View reviewed changes
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request changes command execution display names from folder names to use the "package:foo" format, which provides more consistent and meaningful identification of packages during command execution.

Key Changes:

  • Modified runCommandForWorkspace to attempt to read the package name from pubspec.yaml before falling back to folder names
  • Updated channel name construction logic to avoid redundant display (e.g., "flutter" instead of "flutter (flutter)")
  • Exported tryGetPackageName utility function from fs.ts for use in command execution
  • Updated all related test expectations to reflect the new "package:" prefix format

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/extension/commands/sdk.ts Implements the core logic to prefer package names over folder names, updates parameter naming from shortPath to packageOrFolderDisplayName, and adds smart channel naming to avoid duplication
src/shared/utils/fs.ts Exports the tryGetPackageName function to make it available for use in command execution
src/test/flutter/commands/outdated.test.ts Updates test expectations to use "package:flutter_hello_world" format
src/test/flutter/commands/doctor.test.ts Updates test to expect channel name "flutter" instead of "flutter (flutter)" for the doctor command
src/test/flutter/commands/clean.test.ts Updates test expectations to use "package:" prefix for both root and example packages
src/test/dart/commands/outdated.test.ts Updates test expectations to use "package:hello_world" format

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Nov 17, 2025

Codex Review: Didn't find any major issues. You're on a roll.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@DanTup DanTup force-pushed the use-package-name-for-commands branch from a8997ba to b9816e0 Compare November 17, 2025 17:04
@DanTup DanTup merged commit 284594c into master Nov 17, 2025
3 of 13 checks passed
@DanTup DanTup deleted the use-package-name-for-commands branch November 17, 2025 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

in commands Relates to commands (usually invoked from the command Palette) is enhancement

Projects

None yet

Milestone

v3.124.0

Development

Successfully merging this pull request may close these issues.

Use package name instead of folder in output

2 participants

@DanTup