8000 [AssetMapper] Improve the error message when a downloaded file is missing by jmsche · Pull Request #51337 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[AssetMapper] Improve the error message when a downloaded file is missing #51337

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
merged 1 commit into from
Aug 23, 2023
Merged

[AssetMapper] Improve the error message when a downloaded file is missing #51337

merged 1 commit into from
Aug 23, 2023

Conversation

jmsche
Copy link
Contributor
@jmsche jmsche commented Aug 10, 2023 •
edited by nicolas-grekas
Loading
Q A
Branch? 6.3
Bug fix? no
New feature? no
Deprecations? no
Tickets N/A
License MIT
Doc PR N/A

When a file downloaded using the importmap:require [packageName] --download command is missing, the error message is this one:

The asset "vendor/@hotwired/stimulus.js" mentioned in "importmap.php" cannot be found in any asset map paths.

This PR slightly improves the error message to explain how it can be fixed:

The "vendor/@hotwired/stimulus.js" downloaded asset is missing. Run "php bin/console importmap:require @hotwired/stimulus --download".

@carsonbot carsonbot added this to the 6.4 milestone Aug 10, 2023
@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "6.4" but it seems your PR description refers to branch "6.3".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@jmsche jmsche changed the base branch from 6.4 to 6.3 August 10, 2023 08:21
@jmsche
Copy link
Contributor Author
jmsche commented Aug 10, 2023

Hmm the error spotted by fabbot can't be applied as is. Should I put backticks around the command instead?

AppVeyor, Integration (8.1) & Unit Tests (8.1) failures are unrelated to this PR.

@weaverryan
Copy link
Member

Hmm the error spotted by fabbot can't be applied as is

Couldn't we apply it? It is suggesting:

Run "php bin/console importmap:require "%s" --download"

Are you worried about how the the " around the entire command and. the extra " around the argument look? If so, I think it's ok: the full bin/console command is legal.

@stof
Copy link
Member
stof commented Aug 10, 2023

Anyway, the command will be better with quotes arounds the package name. Depending on the package name, it might require quoting to make the command valid.

@jmsche
Copy link
Contributor Author
jmsche commented Aug 11, 2023

Added quotes around the package name :)

weaverryan
weaverryan approved these changes Aug 15, 2023
View reviewed changes
Copy link
Member
@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

After merge, the message should be updated in #51351 for the new command there.

@jmsche
Copy link
Contributor Author
jmsche commented Aug 16, 2023

@weaverryan will be done :)

@nicolas-grekas nicolas-grekas changed the title [AssetMapper] Improve the error message when a downloaded file is mis… [AssetMapper] Improve the error message when a downloaded file is missing Aug 23, 2023
@nicolas-grekas nicolas-grekas modified the milestones: 6.4, 6.3 Aug 23, 2023
@nicolas-grekas nicolas-grekas force-pushed the improve-importmap-missing-downloaded-file-error-message branch from 2c9834a to 06fb6fe Compare August 23, 2023 20:11
@nicolas-grekas
Copy link
Member

Thank you @jmsche.

@nicolas-grekas nicolas-grekas merged commit 897a054 into symfony:6.3 Aug 23, 2023
@nicolas-grekas
Copy link
Member

Follow up PR welcome

@jmsche
Copy link
Contributor Author
jmsche commented Aug 23, 2023

Sure, I'll update the error message with a PR for the 6.4 branch tomorrow :)

@jmsche jmsche deleted the improve-importmap-missing-downloaded-file-error-message branch August 24, 2023 07:11
@jmsche jmsche mentioned this pull request Aug 24, 2023
Merged
fabpot added a commit that referenced this pull request Aug 30, 2023
@fabpot
minor #51471 [AssetMapper] Improve message when a downloaded asset is…
420a5d2 
… not found (jmsche)

This PR was merged into the 6.4 branch.

Discussion
----------

[AssetMapper] Improve message when a downloaded asset is not found

| Q             | A
| ------------- | ---
| Branch?       | 6.4
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       | N/A
| License       | MIT
| Doc PR        | N/A

This PR improves the exception message introduced in #51337 by mentioning the command introduced in #51351.

Commits
-------

69cae6f [AssetMapper] Improve exception message when a downloaded asset is not found
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weaverryan weaverryan weaverryan approved these changes

Assignees
No one assigned
Labels
AssetMapper Status: Reviewed
Projects
None yet
Milestone
6.3
Development

Successfully merging this pull request may close these issues.
5 participants
@jmsche @carsonbot @weaverryan @stof @nicolas-grekas
0