Show file collision preview in init instead of generic non-empty warning#7870
Show file collision preview in init instead of generic non-empty warning#7870
Conversation
…ollisions - Remove generic "directory is not empty" warning from PromptIfNonEmpty (the warning appeared even when no files would actually be overwritten) - Update promptForDuplicates to show colliding files in red with a clear warning message that only appears when actual file collisions exist - Update tests to reflect the new behavior Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/1dd596ff-15ee-4a94-80eb-dc59f3e40aa5 Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
Agent-Logs-Url: https://github.com/Azure/azure-dev/sessions/1dd596ff-15ee-4a94-80eb-dc59f3e40aa5 Co-authored-by: therealjohn <1501196+therealjohn@users.noreply.github.com>
|
Hi @@copilot. Thank you for your interest in helping to improve the Azure Developer CLI experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
jongio
left a comment
There was a problem hiding this comment.
The approach makes sense - promptForDuplicates already detects exact collisions after the template is staged, so the early blanket warning in PromptIfNonEmpty was firing even when no files would actually collide. Removing it and improving the collision message is the right call.
A few things to address:
-
AZD_ALLOW_NON_EMPTY_FOLDERis now dead: the env var check lived inPromptIfNonEmpty(now a no-op). The constant at line 820, the stripping inInitEnvFileValues(), and the docs inenvironment-variables.mdshould be cleaned up. Users who set it in CI won't break (the prompt is gone for everyone), but dead plumbing is confusing. -
Merge conflicts: the PR can't merge as-is. Needs a rebase onto current
main. -
Nit: existing tests in this file are gradually moving to
t.Context()(8 uses already) per Go 1.26 conventions - the new test code could follow suit instead ofcontext.Background().
| // would collide and lets the user choose to overwrite or keep them. Showing a | ||
| // blanket warning before the collision set is known was confusing: it fired even | ||
| // when no files would actually be overwritten. | ||
| func (i *Initializer) PromptIfNonEmpty(_ context.Context, _ *azdcontext.AzdContext) error { |
There was a problem hiding this comment.
AZD_ALLOW_NON_EMPTY_FOLDER (line 820) was the only consumer of the check that lived here. Now that this is a no-op, that env var has no effect anywhere. Worth cleaning up the constant, the InitEnvFileValues() stripping logic, and docs/environment-variables.md in the same PR.
PromptIfNonEmptyshowed a blanket "WARNING: The current directory is not empty" before the template was even downloaded, so it couldn't know which files would actually collide. This fired even when no files would be overwritten, and never listed the specific collisions.Changes
PromptIfNonEmpty→ no-op: Removed the generic warning and confirmation prompt.promptForDuplicates(called after template staging) already handles collision detection with full file information.promptForDuplicateswarning update: Warning now reads "The current directory is not empty. Initializing an app in this directory may overwrite existing files:" and lists colliding file paths in red (output.WithErrorFormat). No warning is shown when there are zero collisions.TestInitializer_PromptIfNonEmptysimplified to verify no-op. NewTestPromptForDuplicates_ShowsCollisionscovers both the no-collision (silent) and collision (warning + file list) cases.Before/After
When no files collide: no warning shown (previously showed generic warning).
When files collide: