fix: stop announcing a shadowed built-in on every command - #46
Merged
Conversation
Reported from real use: a project with its own init task saw this line before
every command, including ones with nothing to do with init.
$ sputnik drush
Task 'init' in /project/tasks/InitTask.php shadows the built-in init command
...
> ddev exec vendor/bin/drush list
I had treated both discovery messages the same, and they are not the same. A
skipped task means something the author wrote does not work, and that stays true
on the next command - it belongs in the output every time. A shadowed init works
exactly as asked; nothing is broken. Repeating it before unrelated commands is
noise, and it has almost no audience anyway: a project that has tasks has been
initialised already, so the built-in scaffold being unreachable costs nothing.
So discovery now separates warnings from notices. Warnings print always, notices
with -v.
The reporting moved from Kernel::createApplication() into Application::doRun():
-v is parsed by run(), so verbosity is simply not known while the application is
being assembled. The first attempt gated on isVerbose() during assembly and the
notice never appeared, which the E2E test caught.
Verified in the reporting project:
sputnik list (silent)
sputnik list -v Task 'init' in .../InitTask.php shadows ...
a task named 'list' Skipped task 'list' in ... - rename ... (still every run)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reported from real use of 0.2.1. A project with its own
inittask saw this before every command, including ones with nothing to do withinit:My mistake in #45: I treated both discovery messages as the same kind. They are not.
initworks exactly as asked. Nothing is broken.And the shadow notice has almost no audience: a project that has tasks has been initialised already, so the built-in scaffold being unreachable costs nothing. The one moment it helps is when someone is wondering why
sputnik initno longer scaffolds — which is a-vquestion.So discovery separates the two:
getWarnings()prints always,getNotices()with-v. Both still travel through the container cache, since discovery only runs when that cache is cold.The part I got wrong twice
The first attempt gated on
isVerbose()insideKernel::createApplication(), and the notice never appeared even with-v. Reason: Symfony parses-vinrun(), which happens after the application is assembled — verbosity is simply not known at that point.The E2E test caught it, which is why the reporting now lives in
Application::doRun(), where the input has been parsed. That also moved the warnings there, so both come from one place.Verified in the reporting project
And the counter-check that blocked names stay loud — a temporary task named
listin the same project:Still on every run. Temporary task removed afterwards.
Tests
testShadowingIsANoticeNotAWarning— shadowing produces a notice and no warningtestASkippedTaskStaysAWarning— the skip produces a warning and no notice-v, so a future change cannot quietly put it back into every command🤖 Generated with Claude Code
https://claude.ai/code/session_018CTvnzcNYmFgm2HQcm821A